Allow LoRaWAN STM32WL driver debug led to be inverted #14910

hallard · 2021-07-10T15:40:02Z

Summary of changes

Allow LoRaWAN STM32WL debug led to be inverted

Impact of changes

Migration actions required

Documentation

new mbed application STM32WL driver parameter allowing debug led to be reverted (depending on hardware wiring mode)

"target_overrides": {
        "*": {
            "stm32wl-lora-driver.debug_invert": 1
        }

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] 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 @MarceloSalazar

ciarmcom · 2021-07-10T16:00:14Z

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

hallard · 2021-07-11T17:43:51Z

If someone could explain what is preventing the build I can fix it, but the error message is hard to understand from my side.

LMESTM · 2021-07-12T06:56:45Z

@hallard
from what I see here: https://app.travis-ci.com/github/ARMmbed/mbed-os/jobs/523513062
it seems like a formatting issue (trailing white space?), on line 329 of connectivity/drivers/lora/TARGET_STM32WL/STM32WL_LoRaRadio.h:

-// Can be overwriten in the file mbed_app.json with

hallard · 2021-07-12T13:11:14Z

@LMESTM that did it, trailing space at the end of the line.
Thanks

0xc0170

why LED would be inverted?

0xc0170 · 2021-07-13T07:25:26Z

connectivity/drivers/lora/TARGET_STM32WL/STM32WL_LoRaRadio.h

+#undef  DEBUG_LED_ON
+#undef  DEBUG_LED_OFF
+#define DEBUG_LED_ON  0
+#define DEBUG_LED_OFF 1


Can we simplify this? without any undef.

Just assign values based on if its inverted or not.

#if defined(MBED_CONF_STM32WL_LORA_DRIVER_DEBUG_INVERT) && (MBED_CONF_STM32WL_LORA_DRIVER_DEBUG_INVERT == 1) // inverted enabled #define DEBUG_LED_ON 0 #define DEBUG_LED_OFF 1 #else #define DEBUG_LED_ON 1 #define DEBUG_LED_OFF 0 #endif

Yeah sure makes sense Will do that

For me, no need to update this file....
Just use MBED_CONF_STM32WL_LORA_DRIVER_DEBUG_INVERT in STM32WL_LoRaRadio.cpp ?

0xc0170 · 2021-07-13T07:26:05Z

connectivity/drivers/lora/TARGET_STM32WL/mbed_lib.json

@@ -47,6 +47,10 @@
        },
        "debug_tx": {
            "help": "GPIO pin for TX debug"
+        },
+        "debug_invert": {


debug_led_invert might be better

Absolutely the deal is that I wanted to be consistent with already related existing options. It took me some time to understand that ˋdebug_tx` was for led and not a serial. So if I change it it would makes sense to change 2 other (and introduce breaking change). Whatever solution is fine for me

debug_led_invert might be better

Hi

Not sure, as primary goal of the debug GPIO is to easily use logic analyser for example.
Usage of LEDs is only an "extension"

debug_led_invert might be better

Hi

Not sure, as primary goal of the debug GPIO is to easily use logic analyser for example.
Usage of LEDs is only an "extension"

makes sense

hallard · 2021-07-13T07:51:32Z

why LED would be inverted?

Because of hardware side sometimes they are connected in the reversed way

jeromecoutant

Proposition is:

in connectivity/drivers/lora/TARGET_STM32WL/mbed_lib.json, to name the new config "default_debug_value" and set 0 as default
in connectivity/drivers/lora/TARGET_STM32WL/STM32WL_LoRaRadio.cpp, use MBED_CONF_STM32WL_LORA_DRIVER_DEFAULT_DEBUG_VALUE when it was 0 ?

hallard · 2021-07-13T11:41:13Z

Proposition is:

in connectivity/drivers/lora/TARGET_STM32WL/mbed_lib.json, to name the new config "default_debug_value" and set 0 as default

ok

in connectivity/drivers/lora/TARGET_STM32WL/STM32WL_LoRaRadio.cpp, use MBED_CONF_STM32WL_LORA_DRIVER_DEFAULT_DEBUG_VALUE when it was 0 ?

so you mean in this file driving the GPIO that was something like that before

_rf_dbg_rx = 0; // Off 
_rf_dbg_rx = 1; // On

becomes

_rf_dbg_rx = MBED_CONF_STM32WL_LORA_DRIVER_DEFAULT_DEBUG_VALUE 
_rf_dbg_rx = !MBED_CONF_STM32WL_LORA_DRIVER_DEFAULT_DEBUG_VALUE

or something else, not sure to see what you mean.

jeromecoutant · 2021-07-13T11:43:57Z

Yes, that was my idea. Is it working ?
Maybe we could find more appropriate name ?

hallard · 2021-07-13T12:10:25Z

Yes, that was my idea. Is it working ?
Maybe we could find more appropriate name ?

I need to test it

why not MBED_CONF_STM32WL_LORA_DRIVER_DEBUG_LED_OFF, like that default 0 value means Led if Off with value of 0 and if set to 1 it's reversed means 1 to set it off so

_rf_dbg_rx = 0; // Off 
_rf_dbg_rx = 1; // On

becomes

// Off
_rf_dbg_rx = MBED_CONF_STM32WL_LORA_DRIVER_DEFAULT_LED_OFF
// On (really Not Off)
_rf_dbg_rx = !MBED_CONF_STM32WL_LORA_DRIVER_DEFAULT_LED_OFF

Pull request has been modified.

hallard

requested changes done, tested, works as expected with LoRa E5 Breakout

connectivity/drivers/lora/TARGET_STM32WL/mbed_lib.json

jeromecoutant · 2021-07-13T15:09:43Z

connectivity/drivers/lora/TARGET_STM32WL/STM32WL_LoRaRadio.h

@@ -388,4 +388,4 @@ class STM32WL_LoRaRadio : public LoRaRadio {



-#endif /* MBED_LORA_RADIO_DRV_STM32WL_LORARADIO_H_ */
+#endif /* MBED_LORA_RADIO_DRV_STM32WL_LORARADIO_H_ */


remove this patch

sorry bad cut/paste on my side, done

0xc0170 · 2021-07-27T09:30:12Z

Ci started

Squash would be good, I can squash this into one when merging

hallard · 2021-07-27T09:38:17Z

Ci started

Squash would be good, I can squash this into one when merging

Sure it's no problem for squash, even better :-)

mbed-ci · 2021-07-27T09:57:41Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

hallard added 3 commits July 10, 2021 17:34

allow debug led to be inverted

18e4f99

Update STM32WL_LoRaRadio.h

d53f4b2

added comments

3b3b9d7

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

ciarmcom requested review from jeromecoutant, MarceloSalazar and a team July 10, 2021 16:00

ciarmcom added needs: review component: connectivity labels Jul 10, 2021

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

Travis error fix

50f5809

removed trailing space

26bc0d0

0xc0170 previously requested changes Jul 13, 2021

View reviewed changes

jeromecoutant reviewed Jul 13, 2021

View reviewed changes

hallard added 3 commits July 13, 2021 17:05

revert to original

2d9cd8e

changed debug_invert to debug_led_off

18778c9

drive led with mbed conf debug_led_off value

0cfe8fd

hallard commented Jul 13, 2021

View reviewed changes

jeromecoutant reviewed Jul 13, 2021

View reviewed changes

hallard added 2 commits July 13, 2021 17:27

Update STM32WL_LoRaRadio.h

c86f43b

renamed debug_led_off to debug_invert

6f7a56a

hallard requested review from jeromecoutant and 0xc0170 and removed request for a team July 20, 2021 07:07

jeromecoutant approved these changes Jul 21, 2021

View reviewed changes

0xc0170 approved these changes Jul 27, 2021

View reviewed changes

mergify bot added needs: CI and removed needs: work labels Jul 27, 2021

0xc0170 added release-type: patch Indentifies a PR as containing just a patch and removed release-type: feature labels Jul 27, 2021

mergify bot added ready for merge and removed needs: CI labels Jul 27, 2021

0xc0170 merged commit d13aaf4 into ARMmbed:master Jul 29, 2021

mergify bot removed the ready for merge label Jul 29, 2021

hallard deleted the STM32LW_Led branch July 29, 2021 12:40

mbedmain added release-version: 6.14.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Aug 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow LoRaWAN STM32WL driver debug led to be inverted #14910

Allow LoRaWAN STM32WL driver debug led to be inverted #14910

hallard commented Jul 10, 2021 •

edited by 0xc0170

Loading

ciarmcom commented Jul 10, 2021

hallard commented Jul 11, 2021

LMESTM commented Jul 12, 2021

hallard commented Jul 12, 2021

0xc0170 left a comment

0xc0170 Jul 13, 2021

hallard Jul 13, 2021

jeromecoutant Jul 13, 2021

0xc0170 Jul 13, 2021

hallard Jul 13, 2021

jeromecoutant Jul 13, 2021 •

edited

Loading

hallard Jul 13, 2021 •

edited

Loading

hallard commented Jul 13, 2021

jeromecoutant left a comment •

edited

Loading

hallard commented Jul 13, 2021

jeromecoutant commented Jul 13, 2021

hallard commented Jul 13, 2021 •

edited

Loading

hallard left a comment

jeromecoutant Jul 13, 2021

hallard Jul 13, 2021

0xc0170 commented Jul 27, 2021

hallard commented Jul 27, 2021

mbed-ci commented Jul 27, 2021

		@@ -388,4 +388,4 @@ class STM32WL_LoRaRadio : public LoRaRadio {



		#endif /* MBED_LORA_RADIO_DRV_STM32WL_LORARADIO_H_ */
		#endif /* MBED_LORA_RADIO_DRV_STM32WL_LORARADIO_H_ */

Allow LoRaWAN STM32WL driver debug led to be inverted #14910

Allow LoRaWAN STM32WL driver debug led to be inverted #14910

Conversation

hallard commented Jul 10, 2021 • edited by 0xc0170 Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Jul 10, 2021

hallard commented Jul 11, 2021

LMESTM commented Jul 12, 2021

hallard commented Jul 12, 2021

0xc0170 left a comment

Choose a reason for hiding this comment

0xc0170 Jul 13, 2021

Choose a reason for hiding this comment

hallard Jul 13, 2021

Choose a reason for hiding this comment

jeromecoutant Jul 13, 2021

Choose a reason for hiding this comment

0xc0170 Jul 13, 2021

Choose a reason for hiding this comment

hallard Jul 13, 2021

Choose a reason for hiding this comment

jeromecoutant Jul 13, 2021 • edited Loading

Choose a reason for hiding this comment

hallard Jul 13, 2021 • edited Loading

Choose a reason for hiding this comment

hallard commented Jul 13, 2021

jeromecoutant left a comment • edited Loading

Choose a reason for hiding this comment

hallard commented Jul 13, 2021

jeromecoutant commented Jul 13, 2021

hallard commented Jul 13, 2021 • edited Loading

hallard left a comment

Choose a reason for hiding this comment

jeromecoutant Jul 13, 2021

Choose a reason for hiding this comment

hallard Jul 13, 2021

Choose a reason for hiding this comment

0xc0170 commented Jul 27, 2021

hallard commented Jul 27, 2021

mbed-ci commented Jul 27, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

hallard commented Jul 10, 2021 •

edited by 0xc0170

Loading

jeromecoutant Jul 13, 2021 •

edited

Loading

hallard Jul 13, 2021 •

edited

Loading

jeromecoutant left a comment •

edited

Loading

hallard commented Jul 13, 2021 •

edited

Loading