Semtech SX1262 compatibility completion port #1

bdesterBE · 2024-01-04T19:46:54Z

No description provided.

…ter()

ljakob · 2024-01-05T11:03:34Z

src/board/generic_esp32/generic-esp32-board.c

@@ -58,25 +143,21 @@ uint32_t BoardGetRandomSeed( void )
    return (mac_addr_bin[5] << 0) | (mac_addr_bin[4] << 8) | (mac_addr_bin[3] << 16) | (mac_addr_bin[2] << 24);
 }

+#include "nvs.h"
+
 void BoardGetUniqueId( uint8_t *id )


are you sure you want to use the DEVEUI here? all other implementations use something hardware unique - see

https://github.com/Lora-net/LoRaMac-node/blob/99c2e5378dde37e3331bf3efdf9df8f77d4847ee/src/boards/NucleoL152/board.c#L221

Yes I'm glad you called this out, I was iffy on this commit. I am dropping it

I'm not sure of the reason either. Searching online about it, it seems like a bad idea to leave the reset pin floating/doesn't make any difference. I think I am going to leave it as disabled for now for these reasons.

ljakob · 2024-01-05T11:08:14Z

Kconfig

+        help
+            Set the GPIO number used for IRQ (DIO1).
+
+    config LORAWAN_BUSY_GPIO


at least my module has no dedicated BUSY-GPIO as far I can remember - maybe it can be enabled?

@bdesterBE if its indeed an optional thing then you should make this config entry depend on another entry that enables it.

added define guard to limit busy pin to boards that have one.

ljakob · 2024-01-05T11:13:20Z

src/board/generic_esp32/generic-esp32-board.c

+{
+    while(gpio_get_level(lora_spi.busy) == 1);
+}
+
 /*!
 * \brief HW Reset of the radio
 */
 void SX126xReset( void )


I don't know the reason, but in

https://github.com/manuelbl/ttn-esp32/blob/master/src/hal/hal_esp32.c

there is an option to keep reset floating

I'm not sure of the reason either. Searching online it seems to be a bad idea to leave reset pin floating/doesn't make any difference, so I think I will just leave it disabled.

ljakob · 2024-01-05T11:59:32Z

Happy new year,
I've given some comments - but I'll merge it anyway. How far is your implementation? Would the hello world join TTN?
Cheers
Leif

bdesterBE · 2024-01-05T16:03:18Z

Happy new year, I've given some comments - but I'll merge it anyway. How far is your implementation? Would the hello world join TTN? Cheers Leif

Happy new year! Thank you for reviewing, I will fixup/respond to comments today. Please don't merge until you feel it is ready. @cmorganBE may also review.

cmorganBE · 2024-01-05T17:43:54Z

idf_component.yml

@@ -0,0 +1,4 @@
+description: LoRaWAN board port for ESP32


can we make this more clear? this is loramacnode port for esp32 no?

cmorganBE · 2024-01-05T17:45:01Z

src/board/generic_esp32/generic-esp32-board.c

+#define KHZ_PER_MHZ 1000
+#define HZ_PER_MHZ  (HZ_PER_KHZ * KHZ_PER_MHZ)
+
+#define SX1262_SPI_CLOCK_SPEED_MHZ  16


is 16mhz the max speed? if so I'd make another define or constant to define that and then assign that to the clock speed.

change the name of the define to include MAX for clarity

cmorganBE · 2024-01-05T17:45:41Z

src/board/generic_esp32/generic-esp32-board.c

+        .command_bits = 8,
+        .address_bits = 16
+    };
+    ESP_ERROR_CHECK(spi_bus_initialize(SPI2_HOST, &buscfg, SPI_DMA_DISABLED));


how do you know its SPI2?

why disable SPI_DMA?

adding configuration. changed to use DMA, accident.

cmorganBE · 2024-01-05T17:46:14Z

src/board/generic_esp32/generic-esp32-board.c

@@ -81,7 +83,8 @@ void BoardInitPeriph(void)
        .mode = 0,
        .spics_io_num = lora_spi.cs,
        .command_bits = 8,
-        .address_bits = 16
+        .address_bits = 16,
+        .queue_size = 7


you're right, can be 1. changing and added comment.

cmorganBE · 2024-01-05T17:47:27Z

src/board/generic_esp32/generic-esp32-board.c

+
+    ESP_ERROR_CHECK(gpio_set_level(lora_spi.reset, 1));
+
+    // Hold low for at least 100us.


datasheet citation

cmorganBE · 2024-01-05T17:49:03Z

src/board/generic_esp32/generic-esp32-board.c

+    trans.tx_buffer = buffer;
+    trans.length = size * 8;
+
+    SX126xCheckDeviceReady();


what if this never indicates ready? if we are ok with blocking at this point we should note the behavior in the README.md of this component to aid implementers in debugging

added this, and additional detail to the README

cmorganBE · 2024-01-05T17:49:30Z

src/board/generic_esp32/generic-esp32-board.c

+    trans.cmd = RADIO_READ_BUFFER;
+    trans.addr = offset;
+    trans.rx_buffer = buffer;
+    trans.length = size * 8;


why size*8?

added link to docs on this

cmorganBE · 2024-01-05T17:50:36Z

src/board/generic_esp32/generic-esp32-board.c

 static portMUX_TYPE my_spinlock = portMUX_INITIALIZER_UNLOCKED;

+static esp_timer_handle_t irq_timer_handle;


this is a timer for measuring the irq?

It's used to call the stack's internal TimerIRQHandler() function upon timer expiration.

variable name is a little confusing. 'timer_handler' maybe? its unclear where its coming from, internal vs. external etc.

cmorganBE · 2024-01-05T17:53:44Z

.gitmodules

@@ -1,3 +1,3 @@
 [submodule "src/LoRaMac-node"]
 	path = src/LoRaMac-node
-	url = https://github.com/Lora-net/LoRaMac-node.git
+	url = https://github.com/boston-engineering/LoRaMac-node.git


in your fork here can you include output of the linking issue in the git commit log body? have you opened a PR against the upstream repo? we really want to avoid having to use a forked loramac-node if we can avoid it by getting your PR in.

brandester32 · 2024-01-25T03:08:33Z

@ljakob @cmorganBE Ready for review again. This has now been tested with an SX1262 devkit, and successfully sends join requests.

cmorganBE · 2024-01-31T15:56:46Z

src/board/generic_esp32/generic-esp32-board.c

+    int64_t time_us = esp_timer_get_time();
+    uint32_t time_s = (uint32_t)(time_us / US_PER_S);
+
+    *milliseconds = (uint16_t)(time_us * US_PER_MS);


this doesn't look correct, milliseconds appears to be the remainder of time since epoch when you've taken away all of the whole seconds.

…rypt() multiple definitions fix Linking error: .espressif/tools/xtensa-esp32s3-elf/esp-12.2.0_20230208/xtensa-esp32s3-elf/bin/../lib/gcc/xtensa-esp32s3-elf/12.2.0/../../../../xtensa-esp32s3-elf/bin/ld: esp-idf/wpa_supplicant/libwpa_supplicant.a(crypto_mbedtls.c.obj): in function `aes_encrypt': esp/esp-idf/components/wpa_supplicant/esp_supplicant/src/crypto/crypto_mbedtls.c:421: multiple definition of `aes_encrypt'; esp-idf/LoRaMac-node-esp32/libLoRaMac-node-esp32.a(aes.c.obj):src/LoRaMac-node/src/peripherals/soft-se/aes.c:569: first defined here collect2: error: ld returned 1 exit status ninja: build stopped: subcommand failed.

… Remove hard-coding of RegionEU868 definition - should be determined by the application which regions are enabled.

… fork

Replace deprecated register_component() call with idf_component_regis…

36debb2

…ter()

bdesterBE requested a review from cmorganBE January 4, 2024 19:46

bdesterBE self-assigned this Jan 4, 2024

ljakob reviewed Jan 5, 2024

View reviewed changes

cmorganBE requested changes Jan 5, 2024

View reviewed changes

bdesterBE force-pushed the bdd/sx1262_port branch 2 times, most recently from 72ef340 to 5f561b7 Compare January 8, 2024 03:44

bdesterBE force-pushed the bdd/sx1262_port branch 4 times, most recently from b7d7fc9 to c908724 Compare January 25, 2024 02:31

Add configuration files to allow for user configurable SPI parameters

879966d

bdesterBE force-pushed the bdd/sx1262_port branch 2 times, most recently from adeb6d1 to b9b7596 Compare January 25, 2024 03:05

bdesterBE added 2 commits January 24, 2024 22:06

Initialize SPI, and Busy and Reset GPIOs in BoardInitPeriph

bb0757a

Implement SX126x functions for LoRaMac stack

4dc87c1

bdesterBE force-pushed the bdd/sx1262_port branch from b9b7596 to 0d85d0f Compare January 25, 2024 03:06

cmorganBE reviewed Jan 31, 2024

View reviewed changes

bdesterBE added 4 commits February 1, 2024 12:43

Implement SX126x RTC functions for LoRaMac stack

468cdbf

Cmake source files for LoRaMac-node library's US 915MHz band support.…

8d6cf6b

… Remove hard-coding of RegionEU868 definition - should be determined by the application which regions are enabled.

Add some description to README.md

d4b7ed5

bdesterBE force-pushed the bdd/sx1262_port branch from 0d85d0f to d4b7ed5 Compare February 1, 2024 17:43

DO NOT PUSH TO MAIN - Update to use master commit for LoRaMac-node BE…

3da8136

… fork

bdesterBE force-pushed the bdd/sx1262_port branch from 2f541c2 to 3da8136 Compare February 1, 2024 17:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Semtech SX1262 compatibility completion port #1

Semtech SX1262 compatibility completion port #1

bdesterBE commented Jan 4, 2024

ljakob Jan 5, 2024

bdesterBE Jan 8, 2024

bdesterBE Jan 8, 2024

ljakob Jan 5, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

ljakob Jan 5, 2024

bdesterBE Jan 8, 2024

ljakob commented Jan 5, 2024

bdesterBE commented Jan 5, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

cmorganBE Jan 5, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

cmorganBE Jan 8, 2024

cmorganBE Jan 5, 2024

bdesterBE Jan 8, 2024

brandester32 commented Jan 25, 2024

cmorganBE Jan 31, 2024


		ESP_ERROR_CHECK(gpio_set_level(lora_spi.reset, 1));

		// Hold low for at least 100us.

		static portMUX_TYPE my_spinlock = portMUX_INITIALIZER_UNLOCKED;

		static esp_timer_handle_t irq_timer_handle;

Semtech SX1262 compatibility completion port #1

Are you sure you want to change the base?

Semtech SX1262 compatibility completion port #1

Conversation

bdesterBE commented Jan 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljakob commented Jan 5, 2024

bdesterBE commented Jan 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandester32 commented Jan 25, 2024

Choose a reason for hiding this comment