Added LORA-E5 and RAK3172 lorawan modules #14859

hallard · 2021-07-01T17:39:42Z

Summary of changes

Added new lorawan modules with STM32WL, LoRa-E5 and RAK3172 targets
ARMmbed/mbed-os-example-lorawan#232

Impact of changes

Migration actions required

Documentation

You can now use 2 new targets available in targets list

LORA_E5
RAK3172

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@jeromecoutant @ARMmbed/team-st-mcd

ciarmcom · 2021-07-01T18:00:15Z

@hallard, thank you for your changes.
@jeromecoutant @ARMmbed/mbed-os-connectivity @ARMmbed/team-st-mcd @ARMmbed/mbed-os-maintainers please review.

jeromecoutant

Very interesting!
(got only minor comments or questions)

jeromecoutant · 2021-07-02T07:06:05Z

targets/targets.json

+            "MCU_STM32WL"
+        ],
+        "public": false,
+   		"macros_add": [


alignment issue

jeromecoutant · 2021-07-02T07:12:39Z

connectivity/drivers/lora/TARGET_STM32WL/STM32WL_radio_driver.cpp

 /* Sets up radio switch position according to the radio mode */
+#if defined (TARGET_LORA_E5) || defined (TARGET_RAK3172)


I think this list of define will not be "nice" when we will add again more targets? :-)

Proposition:

create a new sub-directory here TARGET_NUCLEO_WL55JC with this current file inside

you can the create TARGET_LORA_E5 and TARGET_RAK3172 with the specific file content
?

Yeah don't like these define either. Subfolder is great, the deal is that I'm not a guru of cmake, so not sure how to tell which dir to include depending on target, I'll try that.

We shouldn't have targets configuration in libraries - these should be hw agnostic.
Instead, you can propose a sw configuration to enable/disable features. This could be used by any target configuration (e.g through mbed_app.json).

done, looks like it was easier than I thought

jeromecoutant · 2021-07-02T07:12:51Z

connectivity/drivers/lora/TARGET_STM32WL/STM32WL_radio_driver.cpp

 /* Sets up radio switch position according to the radio mode */
+#if defined (TARGET_LORA_E5) || defined (TARGET_RAK3172)
+/* This configuration is for RAK3172 or LoRa-E5 modules */
+/* Theese one use only HP mode the LP mode is not connected nor RF Switch ctrl3 */


typo issues

could you precise what do you want me to fix?

jeromecoutant · 2021-07-02T07:13:00Z

connectivity/drivers/lora/TARGET_STM32WL/STM32WL_radio_driver.cpp

+#if defined (TARGET_LORA_E5) || defined (TARGET_RAK3172)
+/* This configuration is for RAK3172 or LoRa-E5 modules */
+/* Theese one use only HP mode the LP mode is not connected nor RF Switch ctrl3 */
+/* Added to avoid declarion in sample code for lorawan example with these mmodules */


same as above

jeromecoutant · 2021-07-02T07:17:30Z

targets/TARGET_STM/TARGET_STM32WL/TARGET_STM32WLE5xC/TARGET_LORA_E5/PinNames.h

+    // I2C signals aliases
+    I2C_SDA = PA_15,
+    I2C_SCL = PB_15,
+
+    // SPI signals aliases
+    SPI_CS   = PB_9,
+    SPI_MOSI = PA_10,
+    SPI_MISO = PB_14,
+    SPI_SCK  = PB_13,
+


Use #define (or remove if not useful)

See https://github.com/ARMmbed/mbed-os/blob/master/hal/include/hal/PinNameAliases.h#L171

jeromecoutant · 2021-07-02T07:18:38Z

targets/TARGET_STM/TARGET_STM32WL/TARGET_STM32WLE5xC/TARGET_LORA_E5/PinNames.h

+    LED1    = PB_5,
+    LED2    = PB_10,
+
+    BUTTON1 = PA_0,


Not sure there is LED and button?

on my board (and SEEED ones) there are but I can add them in override section of the project

I understand.
Maybe you could create a third target for your specific HW ?
@MarceloSalazar

yeah, later :-)

jeromecoutant · 2021-07-02T07:20:38Z

targets/TARGET_STM/TARGET_STM32WL/TARGET_STM32WLE5xC/TARGET_LORA_E5/PeripheralNames.h

+ */
+#ifndef MBED_PERIPHERALNAMES_H
+#define MBED_PERIPHERALNAMES_H
+


As you want, but I think we can move this file at TARGET_STM32WL level to be common for all ?

absolutely, once gain, need knowledge of CMake, will try.

are you ok to move 3 files PeripheralNames.c PeripheralPins.h system_clock.c to TARGET_STM32WL and keep only PinNames.h in target folder?

PeripheralPins.c : no, specific for each target (you can manually disable or not a pin, you can add comment, etc...)

PeripheralNames.h : yes

system_clock.c: why not, as SetSysClock is weak

jeromecoutant · 2021-07-02T07:22:11Z

About Travis error, it seems that you need to rebase on the latest master
@0xc0170

jeromecoutant · 2021-07-02T07:26:16Z

FYI @ludoch-stm

MarceloSalazar

Note this isn't considered an officially support board with valid board ID.

You can add Mbed OS support following the guidelines for Custom and community boards

Pull request has been modified.

hallard · 2021-07-02T10:37:56Z

@MarceloSalazar does this means that this PR has no chance to be merged? These 2 modules are not really a board but just a module to be put on custom/pro boards. Boards made with this module can be used as custom boards of course but target is always a STM32WL one. The Difference with STM32WL55 is that these one are single core STM32WLE5.

MarceloSalazar · 2021-07-02T15:11:47Z

@hallard I'm afraid we can't accept this PR as new target as it's not considered an officially supported board.

You may still want to contribute with changes in Mbed OS to let us configure specific features in libraries and support custom boards/modules.

Please check the docs for enabling custom boards.

@jeromecoutant let us know if you have some ideas on how support custom boards based on ST MCUs.

jeromecoutant · 2021-07-02T15:37:30Z

OK...

@hallard keep all patches except the 2 directories:

targets/TARGET_STM/TARGET_STM32WL/TARGET_STM32WLE5xC/TARGET_LORA_E5
targets/TARGET_STM/TARGET_STM32WL/TARGET_STM32WLE5xC/TARGET_RAK3172

In targets.json, keep only "MCU_STM32WLe5xC" definition.

I will save the missing files in a ST mbed repo soon.

jeromecoutant · 2021-07-02T16:21:35Z

Here is the new repo : https://github.com/ARMmbed/stm32customtargets

Feel free to make PR there as well!

@ARMmbed/team-st-mcd

hallard · 2021-07-03T13:06:04Z

Awesome, already done that but on my work computer, will finish all by monday, nice work.

By the way, just curious, where I need to put the weak function for rf swtich configuration for these 2 boards?

hallard · 2021-07-05T11:10:32Z

Closed, as discussed on this thread, now added into #14872 and custom target will be created into
https://github.com/ARMmbed/stm32customtargets as asked by @MarceloSalazar and @jeromecoutant

Added LORA-E5 and RAK3172 lorawan modules

1ed4cff

hallard mentioned this pull request Jul 1, 2021

Target NUCLEO WL55JC does not compile ARMmbed/mbed-os-example-lorawan#232

Closed

ciarmcom added the release-type: feature label Jul 1, 2021

ciarmcom requested review from jeromecoutant and a team July 1, 2021 18:00

ciarmcom added needs: review component: connectivity devices: st labels Jul 1, 2021

mergify bot added needs: work and removed needs: review labels Jul 1, 2021

hallard added 3 commits July 1, 2021 23:15

format for travis pass

2a1e103

cosmetic

3e8e052

device_name in target for travis

da1b28a

jeromecoutant reviewed Jul 2, 2021

View reviewed changes

hallard added 8 commits July 2, 2021 11:12

Merge branch 'master' into lora-e5

43185d8

aligment

ba833f4

moved up to to STM32WL folder

cabc95b

moved up to to STM32WL folder

b7aecc7

moved up to to STM32WL folder

91e5abe

moved up to to STM32WL folder

48ccc9f

specific target subfolder for radio driver

637ffe3

moved up to to STM32WL folder

b1de779

MarceloSalazar previously requested changes Jul 2, 2021

View reviewed changes

use define

f8b571e

added STM32WLE5JCIx

b227d3a

hallard added 6 commits July 5, 2021 09:56

removed board def from driver

0c292a8

removed custom boards

80d77d5

Create STM32WL_radio_driver.cpp

0a06bc5

Update STM32WL_radio_driver.cpp

95cd992

removed LoRa-E5 and RAK3172

023b0bc

Update STM32WL_radio_driver.cpp

ce32eb7

hallard mentioned this pull request Jul 5, 2021

Add STM32WLE5 for custom targets LORA_E5 and RAK3172 #14872

Merged

hallard closed this Jul 5, 2021

mergify bot removed component: connectivity devices: st needs: work release-type: feature labels Jul 5, 2021

hallard deleted the lora-e5 branch July 8, 2021 15:47

		/* Sets up radio switch position according to the radio mode */
		#if defined (TARGET_LORA_E5) \|\| defined (TARGET_RAK3172)

Added LORA-E5 and RAK3172 lorawan modules #14859

Added LORA-E5 and RAK3172 lorawan modules #14859

Conversation

hallard commented Jul 1, 2021 • edited Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

@jeromecoutant @ARMmbed/team-st-mcd

ciarmcom commented Jul 1, 2021

jeromecoutant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromecoutant Jul 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarceloSalazar Jul 2, 2021 • edited Loading

Choose a reason for hiding this comment

hallard Jul 2, 2021 • edited Loading

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

jeromecoutant Jul 2, 2021 • edited Loading

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

hallard Jul 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromecoutant commented Jul 2, 2021

jeromecoutant commented Jul 2, 2021

MarceloSalazar left a comment • edited Loading

Choose a reason for hiding this comment

hallard commented Jul 2, 2021

MarceloSalazar commented Jul 2, 2021

jeromecoutant commented Jul 2, 2021

jeromecoutant commented Jul 2, 2021

hallard commented Jul 3, 2021 • edited Loading

hallard commented Jul 5, 2021 • edited Loading

hallard commented Jul 1, 2021 •

edited

Loading

jeromecoutant Jul 2, 2021 •

edited

Loading

MarceloSalazar Jul 2, 2021 •

edited

Loading

hallard Jul 2, 2021 •

edited

Loading

jeromecoutant Jul 2, 2021 •

edited

Loading

hallard Jul 2, 2021 •

edited

Loading

MarceloSalazar left a comment •

edited

Loading

hallard commented Jul 3, 2021 •

edited

Loading

hallard commented Jul 5, 2021 •

edited

Loading