Add support for STM32WL integrated transceivers by matthijskooijman · Pull Request #649 · jgromes/RadioLib

matthijskooijman · 2022-12-20T22:00:12Z

This fixes #588 by adding support for STM32WL transceivers.

As discussed in that issue, this adds a new radio class to support the radio integrated into the STM32WL series of microcontrollers. The implementation is nearly complete, but there are a couple of things that need some additional review and some changes before they are ready to merge. Those commits have been marked with "Draft", and details of what is needed are given in the commit messages.

This PR is structured as follows:

The first three commits generalize the RfSwitch handling to support more than two pins and more different operation modes.
Then one commit that prepares SX126x for adding a new subclass (this splits off one change, some other smaller changes to SX126x are included in the STM32WLx commit later on).
The rest of the commits add the STM32WLx implementation. This starts with a big commit and then some smaller ones, that could probably have been a single commit but I hope that splitting them like this makes everything easier to review (and we can consider squashing them later, or leaving them split for the commit history).

~~This PR relies on some changes in the Arduino Core that are not yet merged. To compile the code, you need a git version of the core with stm32duino/Arduino_Core_STM32#1839 applied.~~

Edit: This PR needs version 2.4.0 of the STM32 Arduino core at https://github.com/stm32duino/Arduino_Core_STM32

This PR contains some things that were not discussed in #588 yet, or implemented differently:

The class name is now STM32WLx, because STM32WL was already defined as a macro somewhere. It derives from SX1262 instead of SX126x, since that allowed reusing some code after all.
The RFSwitch table code was done in a (hopefully) more portable way (not relying on pin numbers and states being integers) and without templates. It is now limited to 3 pins (but could be extended in the future with some work) and an unlimited amount of modes.
I've added an STM32WLx_Module class, which is a subclass of Module that handles overriding some of the callbacks and neatly hides all the (virtual) pin numbers and SPI configuration from the user (since the radio is internally hardwired, there is no need for the user/sketch to configure any of this).

I've tested basic transmission and reception, both regular and using interrupts on a Nucleo WL55JC1 board. This PR duplicates these four examples from SX126x for STM32WLx, since the initialization needs some significant changes. I haven't modified all examples, though, since that would result in more duplicate code (but I guess that duplicating the other five examples might be useful, since I'll need to test these things anyway, at least CAD and FSK, maybe also PingPong and Settings). I have tested interoperability with other radios early in the implementation (using an SX127x radio), I'll doublecheck that once the implementation is further finalized.

As shown in the new files' headers, copyright for this contribution is assigned to STMicroelectronics, but made available under the MIT license matching the existing license of this library.

Here's a bit of documentation that was removed from the code during review of the PR, but might be good to put on the wiki or elsewhere later:

RF Switch and low-power vs high-power transmissions

One notable difference with the %SX126x radios is that this radio
essentially combines the %SX1261 and %SX1262 by integrating both the
low-power (LP) and high-power (HP) amplifier. Because both output on
two different pins, the external RF switch on the board must be told
(through GPIO pins) which mode is used so it can connect the proper
output (or input, for RX) to the antenna.

Because of this, and because the DIO2 pin (which can be used on
external SX126x radios to let the radio handle RF switch control) is
not available externally on the STM32WL, you must configure an
RfSwitch table when using this radio.

Whether to use the LP or HP mode is automatically decided by the
setOutputPower() method, see its documentation for details.

Some boards might only have circuitry for just the low-power or
high-power transmission paths. For these boards, just omit the
unsupported mode from the table and setOutputPower will adjust its
behavior accordingly.

See Module::setRfSwitchTable() for general info on how to call this
method. Note that because of the LP vs HP output, this radio defines
its own OpMode_t constants, which split the TX mode into two versions
and the example in the Module::setRfSwitchTable() method documentation
does not apply.

Instead, consider this example that sets up the RfSwitch as needed for
the Nucleo WL55JC1 board:

// In global scope, define the pin array and mode table
static const RADIOLIB_PIN_TYPE rfswitch_pins[] =
                         {PC3,  PC4,  PC5};
static const Module::RfSwitchMode_t rfswitch_table[] = {
  {STM32WLx::MODE_IDLE,  {LOW,  LOW,  LOW}},
  {STM32WLx::MODE_RX,    {HIGH, HIGH, LOW}},
  {STM32WLx::MODE_TX_LP, {HIGH, HIGH, HIGH}},
  {STM32WLx::MODE_TX_HP, {HIGH, LOW,  HIGH}},
  STM32WLx::END_OF_MODE_TABLE,
};

void setup() {
  ...
  // Then in setup (before calling begin), pass them to radiolib
  radio.setRfSwitchTable(rfswitch_pins, rfswitch_table);
  ...
}

Note that the setRfSwitchTable() method should be called before the
begin() method, to ensure the radio knows which modes are supported
during initialization.

jgromes · 2022-12-21T12:10:44Z

This looks very good!

Regarding the examples, I'm fine with just leaving the Rx/Tx ones. CAD could be useful as well, but FSK and Settings, I don't think those are needed unless the behavior of the integrated transceiver differs significantly in this regard.

One thing that will still need to be set up is CI (it doesn't make sense to built the STM32WL examples on other platforms), but that can be taken care of by using a pattern that will skip all examples named STM32WL*. The same mechanism was used in previous versions of the library e.g. for HTTP on ESP32 (before that was dropped). It is probably better if I handle that.

I'll review and let you know. Is there an estimate when a new release of the STM32 core with this support can be made available?

matthijskooijman · 2022-12-21T17:22:09Z

This looks very good!

Thanks!

Regarding the examples, I'm fine with just leaving the Rx/Tx ones. CAD could be useful as well, but FSK and Settings, I don't think those are needed unless the behavior of the integrated transceiver differs significantly in this regard.

Ah, I just noticed that FSK and Settings are really just for documentation, and not full examples, so no real point in copying those indeed. I'll copy the CAD examples in a future push, because I at least want to test that they work :-)

One thing that will still need to be set up is CI (it doesn't make sense to built the STM32WL examples on other platforms), but that can be taken care of by using a pattern that will skip all examples named STM32WL*. The same mechanism was used in previous versions of the library e.g. for HTTP on ESP32 (before that was dropped). It is probably better if I handle that.

Ah, I see. I just pushed a commit attempting to handle that, not sure if this is what you had in mind?

I'll review and let you know. Is there an estimate when a new release of the STM32 core with this support can be made available?

@fpistm is finalizing the review now, so should be somewhere in the coming days.

fpistm · 2022-12-21T17:28:46Z

Hi @jgromes

Is there an estimate when a new release of the STM32 core with this support can be made available?

Yes, it is available 😉
https://github.com/stm32duino/Arduino_Core_STM32/releases/tag/2.4.0

jgromes

@matthijskooijman I left a coupel of comments in the review - overall there's nothing major that I can see.

I also fixed the CI and slightly cleaned up the examples. Thinking about those, it might be better to move them from the SX126x folder to a new STM32WLx folder, just for clarity.

src/Module.h

src/modules/SX126x/STM32WLx.h

src/modules/SX126x/STM32WLx_Module.h

src/modules/SX126x/SX1262.cpp

src/modules/SX126x/SX126x.cpp

src/Module.h

src/Module.cpp

src/modules/SX126x/SX126x.h

src/modules/SX126x/STM32WLx_Module.cpp

matthijskooijman · 2022-12-23T12:49:23Z

@matthijskooijman I left a coupel of comments in the review - overall there's nothing major that I can see.

Cool, thanks. Also for the quick and solid review. So far it's been a pleasure collaborating with you on this project :-)

I've responded to some of your comments, the other (obvious) ones I'll fix later today.

I also fixed the CI and slightly cleaned up the examples.

Ah, I do remember typing that filter, not sure why it didn't end up in the commit. Thanks for adding it :-)

Thinking about those, it might be better to move them from the SX126x folder to a new STM32WLx folder, just for clarity.

Sounds reasonable, I'll do that.

I'll try to make further changes to the PR by adding new commits to simplify review, but I'm inclined to squash some things together later (e.g. I could squash in your commits into mine already) unless you prefer not to?

jgromes · 2022-12-23T19:27:37Z

@matthijskooijman feel free to squash commits, that's fine.

matthijskooijman · 2022-12-24T16:16:18Z

I've addressed all comments in new fixup commits and squashed some of your commits into previous commits, so I did a force push. I've also moved the examples into their own directory, but I already squased that into the original commits.

I've left the comments open for you to resolve once you're satisfied with the resolution.

Once everything is resolved, there's a few more changes I want to make to the PR:

Apply bd66422 (use \copydoc for setRfSwitchPins) to all radios
Add setRfSwitchTable to all radios that currently have setRfSwitchPins.
Squash fixups and the verbatim example copy commit.

I also want to modify setRfSwitchState to return a status code (so you get an error when a mode is omitted from the table but is used, instead of the radio silently not working or maybe even be damaged), but that requires a bit more work on all callsites (which I'm planning to outsource to ChatGPT, but the scripting around that is turning into a project of its own, so that will take a bit more time).

matthijskooijman · 2023-01-06T14:25:31Z

@jgromes, did you get a chance to look at my last changes yet? I don't want to put pressure, but I'm holding off the last couple of changes (which are mostly more of the same thing) until you have given your blessing on the current changes to prevent double work :-)

jgromes · 2023-01-07T10:19:05Z

Sorry, I completely missed the changes over the holidays :/ Everything is OK from my point of view (hence I resolved all the conversations), so feel free to start the cleanup, squashing etc.

All radios that support RfSwitch define this method that simply forwards to the `Module::setRfSwitchPins()` method. Previously, all these methods duplicated the documentation as well, but this uses the doxygen \copydoc to remove this duplication.

This defines operation modes (IDLE, RX and TX) and allows defining up to to three pins to be controlled. For each mode a value can be specified for each pin a table. Compared to the previous handling, this: - Allows up to three pins instead of only two. - Gives more control over output pin values (e.g. to simply change polarity or support more complex control logic). In addition, the modes are treated as opaque by the Module code, allowing radio classes to define their own modes if needed. Some notes regarding the implementation: - The number of pins is limited at three, since most boards seem to need only two pins and only the Nucleo STM32WL55 board needs three. If more pins are needed in the future, the setRfSwitchTable() can be overloaded to accept either a 3-element or e.g. 4-element pins array, to allow new and old code to work as-is. Note that there is a RFSWITCH_MAX_PINS constant defined, but it is not recommended for sketches to use this constant when defining a rfswitch pins array, to prevent issues when this value is ever increased and such an array gets extra zero elements (that will be interpreted as pin 0). Note that this is not a problem for the RfSwitchMode_t values array, since any extra values in there will only be used if a valid pin was set in the pins array. - The pins array is passed by reference, so the compiler complains if the array passed is not the expected size. Since a reference to an array without a length is not supported (at least not by the gcc 7 used by the AVR core - gcc 10 for STM32 seems to accept it), the table array is passed as a pointer instead (but because arrays and pointers are reasonably interchangeable, the caller does not see the difference). - The existing setRfSwitchPins() method is still supported as before. Internally it creates a table with the right values and pins and passes those to setRfSwitchTable. - For easier review, this commit does not modify all calls to setRfSwitchState() in all radio modules yet, but has a compatibility wrapper to delay this change until the next commit. Similarly, the setRfSwitchTable() method is now defined on Module only, a wrapper for it will be defined in all radios that already have the setRfSwitchPins() wrapper in another commit. - To allow future radios to define any number of modes, the modes table does not have a fixed length, but instead is terminated by a special value. This is a bit fragile (if the terminator is omitted, the code will read past the end of the array), but rather flexible. One alternative to this approach would be to make setRfSwitchTable a template that deduces the array size from a template argument and then stores the size explicitly, but using templates probably reduces code clarity.

This gives all radios that use an rfswitch (i.e. have a setRfSwitchPins() wrapper already) a wrapper method for setRfSwitchTable() too. This wrapper just calls the same method on Module, to make it easier for sketches to use it.

This removes the compatibility wrapper and applies the following replacements: sed -i 's/setRfSwitchState(LOW, LOW)/setRfSwitchState(Module::MODE_IDLE)/' src/modules/*/*.cpp sed -i 's/setRfSwitchState(HIGH, LOW)/setRfSwitchState(Module::MODE_RX)/' src/modules/*/*.cpp sed -i 's/setRfSwitchState(LOW, HIGH)/setRfSwitchState(Module::MODE_TX)/' src/modules/*/*.cpp

This prepares for adding a STM32WLx subclass later.

…#588) This is a nearly complete implementation, except that the Dio1 interrupt is not yet supported (this will be added in a subsequent commit to simplify review). This fixes jgromes#588.

Because this interrupt is not connected to a GPIO or even the EXTI unit, but directly to the MCU NVIC, the regular attachInterrupt flow cannot be used. Instead, this lets STM32WLx override the setDio1Action() and clearDio1Action() methods to register the radio IRQ directly. Note that it would have been nicer to implement this by overriding attachInterrupt in STM32WLx_Module, but that is never called for virtual pins (because the STM32Duino digitalPinToInterrupt() macro does not work for virtual pins). In addition, because the NVIC is level-sensitive and the code expects edge-sensitive interrupts (for GPIO, the EXTI module in front of the NVIC makes the interrupts edge-sensitive), this adds some additional handling around the user-registered interrupt callback. To prevent the ISR from triggering over and over again (and also to not require SPI transactions in the ISR to clear the IRQ flags inside the radio), the interrupt is disabled in the NVIC whenever it is trigered. Then, whenever the IRQ flags *are* cleared in the radio, the IRQ is enabled in the NVIC again. This requires making the SX126x::clearIrqStatus() method virtual so STM32WLx can override it.

This modifies SX1262::begin and beginFSK to call setOutputPower after applying the workaround, so that can undo the workaround if needed.

Now that clearIrqStatus also clears the pending flag, reading the IRQ flag no longer has to, making this more reliable.

This checks for some system macros to be set, but also includes the doxygen build to generate documentation. This reuses the RADIOLIB_EXCLUDE_STM32WLX that the code already checks for to prevent having to duplicate this macro check in four places. To detect running inside doxygen, this configures doxygen to predefine a DOXYGEN macro (it seems no such macros are defined by default by doxygen).

Co-Authored-By: jgromes <jan.gromes@gmail.com>

This adds a build for the Nucleo WL55JC1 board, and ensures that the STM32WLx examples are skipped on all other boards. To slightly generalize the list of boards, this pushes the pnum option for the black pill into the boards list, instead of setting it into the options variable (even though it is technically a board option and not the board itself, conceptually it is part of the selected board and these values are just concatenated, so it can go in either place). Co-Authored-By: jgromes <jan.gromes@gmail.com>

matthijskooijman · 2023-01-10T20:15:34Z

I've pushed another update to this PR with further cleanups and remaining TODOs. As far as I'm concerned, it is now ready to be merged.

Apart from rebasing on top of master and squashing commits, I've made the following actual changes to the code compared to my previous push:

Applied commit 90b28d7 (Remove duplicated setRfSwitchPins documentation) to all radios
Applied commit 3779faf (Add setRfSwitchTable() wrapper methods) to all radios
Added CAD examples in commit e116a20 ([STM32WLx] Add examples)
Updated interrupt examples to remove enableInterrupt like in master in commit e116a20 ([STM32WLx] Add examples)
Fixed STM32WLx::setOutputPower in commit 5e47d94 ([STM32WLx] Add module for STM32WL MCUs with integrated radio). It would previously use the LP amplifier up to 15dBm, based on docs that said it should support up to 15dBm. In testing, I noticed that it did not work, since 15dBm actually requires special handling (configuring 14dBm and then using some different PA config values). I did not add the special handling, but just limited to 14dBm, just like the existing SX1261 code does.

While testing I also noticed some weirdness with the HP amplifier power configuration (changing the TX power on one board would not change the RSSI on another board in the way I would expect, while it worked ok for the LP amplifier), but I suspect that that might have been a problem in my measurement setup, or maybe an existing problem with how the HP/SX1262 amplifier is configured, since I observed the same problem with an external SX1262 transceiver on an Arduino Uno. I want to further investigate this when I have some more time, but I do not think this is really related to this PR, so it should not need to block this PR. Note to self: the datasheet is a bit vague on whether PaConfig needs changes to change the output power, but it seems that just modifying the power value should be enough according to semtech, since the PR that changes PaConfig to the nearest "optimal" value seems to be rejected: Lora-net/LoRaMac-node#999

I previously also proposed modifying setRfSwitchState to return a value, but my chatGPT refactor script for that is taking a bit more work that anticipated. Since this just a nice addition that is not required in any way to use this PR, I suggest also not letting this block this PR, I'll finish this in the near future and submit a new PR for it.

With that, I think this PR should be mergeable as it is now.

thebentern · 2023-01-11T15:17:37Z

This is pretty exciting! Over on the Meshtastic project we've had our eye on some of boards that utilize the STM32WLx and are looking forward to the RadioLib support. :-)

jgromes · 2023-01-11T20:46:52Z

@matthijskooijman thanks for the additions, I'll try to review everything one last time over the next few days :)

jgromes

From my point of view, everything is looking fine to be merged.

@matthijskooijman I also want to thank you, personally - this is a major contribution to RadioLib, and it is obvious a lot of personal time and dedication went into it. So, thank you, this project wouldn't be what it is now without community support and contributions.

Also, big thanks to @fpistm and everyone at STMicroelectronics who helped with this as well!

matthijskooijman · 2023-01-15T11:05:21Z

\o/ Thanks!

it is obvious a lot of personal time and dedication went into it. So, thank you, this project wouldn't be what it is now without community support and contributions.

As for the "personal time", I have to forward those thanks to ST who have been paying for my time working on this. For the dedication, I can take credit myself ;-)

In any case, I'd like to also thank you and previous contributors - by building on your previous work, we now have a very feature-rich library for point-to-point communication on STM32WL, much better than if we would have written something from scratch in the same time :-) Also, I very much enjoyed the collaboration so far, your dedication and technical skills made it a pleasure to work on this for me.

Looking at the future: I'm happy to stay involved with maintaining the STM32WL support, so if you run into problems in this area, feel free to tag me in issues and I'll try to chip in (no promises though - the ST project is finished and my personal time is always overbooked ;-p).

fpistm · 2023-01-15T11:19:35Z

Thank you all :)

amirna2 · 2023-01-27T07:38:40Z

Thank you for this great contribution. Is this compatible with the STM32WLE55JC?
I purchased a couple of Seeed Studio E5 development boards for a project and I would like to first prototype it in the Arduino ecosystem and RadioLib.
https://www.seeedstudio.com/LoRa-E5-Dev-Kit-p-4868.html?queryID=7d286374878ee8330deed17fa1dd7440&objectID=4868&indexName=bazaar_retailer_products

acceleroto · 2023-01-27T07:48:52Z

Thank you for this great contribution. Is this compatible with the STM32WLE55JC?
I purchased a couple of Seeed Studio E5 development boards for a project and I would like to first prototype it in the Arduino ecosystem and RadioLib.

I've been meaning to try this out with my Wio-E5 project, but I haven't made the time yet. Looking through the code, there might be a hiccup or two, but it seems like it should work.

I haven't verified this, but you might hit a speedbump if you set the E5's output power to 14 dBm. From my code browsing, 17, 20 or 22 seemed like they'd work though. Modifying the RFSwitch table might fix that though. Again, I haven't tried yet.

jgromes · 2023-01-27T20:37:46Z

@amirna2 @acceleroto please keep discussions in separate threads, unless you have something specific to this PR. Thanks!

matthijskooijman mentioned this pull request Dec 20, 2022

Adding support for STM32WL integrated transceivers #588

Closed

fpistm mentioned this pull request Dec 21, 2022

GNSE/STM32WL supported platforms TheThingsIndustries/generic-node-se#206

Open

jgromes requested changes Dec 22, 2022

View reviewed changes

matthijskooijman force-pushed the add-stm32wl branch from a841a82 to feede0b Compare December 24, 2022 16:09

matthijskooijman and others added 13 commits January 9, 2023 09:46

Add setRfSwitchTable() wrapper methods

3779faf

This gives all radios that use an rfswitch (i.e. have a setRfSwitchPins() wrapper already) a wrapper method for setRfSwitchTable() too. This wrapper just calls the same method on Module, to make it easier for sketches to use it.

[SX126x] Allow subclasses to change the TX mode used

da6c3f6

This prepares for adding a STM32WLx subclass later.

[STM32WLx] Add module for STM32WL MCUs with integrated radio (jgromes…

5e47d94

…#588) This is a nearly complete implementation, except that the Dio1 interrupt is not yet supported (this will be added in a subsequent commit to simplify review). This fixes jgromes#588.

[STM32WLx] Apply PA clamp workaround for HP only

cd138dd

This modifies SX1262::begin and beginFSK to call setOutputPower after applying the workaround, so that can undo the workaround if needed.

[STM32WLx] Make reading IRQ flag more reliable

3e2810c

Now that clearIrqStatus also clears the pending flag, reading the IRQ flag no longer has to, making this more reliable.

[STM32WLx] Add examples

e116a20

Co-Authored-By: jgromes <jan.gromes@gmail.com>

Added new keywords

ba05317

matthijskooijman force-pushed the add-stm32wl branch from feede0b to ba05317 Compare January 10, 2023 20:15

matthijskooijman marked this pull request as ready for review January 11, 2023 18:41

jgromes approved these changes Jan 14, 2023

View reviewed changes

jgromes merged commit 7267e5f into jgromes:master Jan 14, 2023

fpistm mentioned this pull request May 30, 2023

Assync OTA stm32duino/STM32LoRaWAN#14

Closed

Conversation

matthijskooijman commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

RF Switch and low-power vs high-power transmissions

Uh oh!

jgromes commented Dec 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthijskooijman commented Dec 21, 2022

Uh oh!

fpistm commented Dec 21, 2022

Uh oh!

jgromes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthijskooijman commented Dec 23, 2022

Uh oh!

jgromes commented Dec 23, 2022

Uh oh!

matthijskooijman commented Dec 24, 2022

Uh oh!

matthijskooijman commented Jan 6, 2023

Uh oh!

jgromes commented Jan 7, 2023

Uh oh!

matthijskooijman commented Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thebentern commented Jan 11, 2023

Uh oh!

jgromes commented Jan 11, 2023

Uh oh!

jgromes left a comment

Choose a reason for hiding this comment

Uh oh!

matthijskooijman commented Jan 15, 2023

Uh oh!

fpistm commented Jan 15, 2023

Uh oh!

amirna2 commented Jan 27, 2023

Uh oh!

acceleroto commented Jan 27, 2023

Uh oh!

jgromes commented Jan 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

matthijskooijman commented Dec 20, 2022 •

edited

Loading

jgromes commented Dec 21, 2022 •

edited

Loading

matthijskooijman commented Jan 10, 2023 •

edited

Loading