Complete first version #3
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
matthijskooijman
force-pushed
the
complete-first-version
branch
3 times, most recently
from
d9bd9ec
to
3876108
Compare
fpistm
reviewed
Sep 14, 2022
This adds the .astylerc configuration in the project root, so you can
also run astyle locally. Unfortunately, astyle does not pick it up
automatically and always needs file patterns specified on the
commandline, so to run astyle you need something like:
astyle --project=.astylerc --recursive '*.c' '*.h' '*.ino'
(you can also set `ARTISTIC_STYLE_PROJECT_OPTIONS=.astylerc` in your
environment and omit `--project`)
These are various pieces of glue that provide a "board support package" to the STM32CubeWL codebase. These mostly contain configuration for the STM32CubeWL code or initialization for HAL components used by STM32CubeWL or other parts of the code. Note that these files reference some STM32CubeWL files that are not yet added to the repo yet, these will be added later (but ordering things like this makes it easier to add those files later and update their references to these glue files directly). Most of these files are based on various template files from the LoRaWAN or SubGhz middlewares in STM32CubeWL and licensed under the BSD-3-Clause license. The origin of the files added in this commit is as follows (original paths reference to files from STM32CubeWL). - src/Glue/radio_conf.h Based on Middlewares/Third_Party/SubGHz_Phy/Conf/radio_conf_template.h - src/Glue/mw_log_conf.h Based on Middlewares/Third_Party/SubGHz_Phy/Conf/mw_log_conf_template.h - src/Glue/lorawan_conf.h Based on Middlewares/Third_Party/LoRaWAN/Conf/lorawan_conf_template.h - src/Glue/timer.h Largely taken verbatim from Middlewares/Third_Party/LoRaWAN/Conf/timer_template.h - src/Glue/se-identity.h Based on ./Middlewares/Third_Party/LoRaWAN/Conf/se-identity_template.h - src/Glue/subghz.[ch] Based on ./Middlewares/Third_Party/SubGHz_Phy/Conf/subghz_template.[ch] - src/Glue/radio_board_if.h Based on Projects/NUCLEO-WL55JC/Applications/LoRaWAN/LoRaWAN_End_Node/LoRaWAN/Target BSD-licensed version provided privately by ST. - src/Glue/utilities_conf.h src/Glue/systime.h src/Glue/radio_board_if.c New implementation.
This script copies selected files from the STM32CubeWL package (unpacked zip or git clone) into this library as needed. It can be used for the initial copy as well as for later updates.
This script can be used to, after an update, fix up various include paths. The STM32CubeWL code assumes a lot of files are in the include path, but this is not the case in the Arduino build. So this script modifies the STM32CubeWL files (and any other source files) to resolve any include files using relative paths for any header files that are available within this library. This script is intended to be ran whenever update.sh is ran.
This used the previously added update.sh script to add the relevant bits of the STM32CubeWL code verbatim, and uses the fix-includes.py script to fix the include directives. No other changes were made to these files. This code was downloaded from https://www.st.com/en/embedded-software/stm32cubewl.html as version 1.2.0 was not published to https://github.com/STMicroelectronics/STM32CubeWL yet.
The main part of this commit is the timer_if.c code, which implements
UTIL_TimerDriver using the RTC and direct HAL calls. rtc.c/h handles RTC
initialization.
This commit uses a BSD-licensed version of these files, which was
privately provided by ST. These files are identical to the files from
STM32CMubeWL 1.2.0, except for the license header.
STM32CubeWL/Projects/NUCLEO-WL55JC/Applications/LoRaWAN/LoRaWAN_End_Node/Core/*/timer_if.[ch]
STM32CubeWL/Projects/NUCLEO-WL55JC/Applications/LoRaWAN/LoRaWAN_End_Node/Core/*/rtc.[ch]
The files added in this commit have only been modified using
fix-include.py, and by adding an alarm IRQ handler to rtc.c.
main.h is a very much stripped down version of that file, containing
just some RTC settings used by both other files.
This class is the main entrypoint for the Arduino API, wrapping the STM32CubeWL/LoRaMAC-Node API. This adds an initial version that has roughly the right shape, but is still incomplete and does not properly mimic the MKRWAN API yet (in particular is non-blocking everywhere and does not implement Stream I/O yet).
This is just minimal and incomplete example.
This can be enabled be defining `RTIF_DEBUG`. To make this work without modifying `timer_if.c`, this adds a tiny `sys_app.h` file that just defines a dummy UTIL_ADV_TRACE_COND_Fsend macro that forwards to the MW_LOG function.
This is just the default file generated by `doxygen -g` (version 1.9.1).
This is a rather big commit that declares all methods that MKRWAN and MKRWAN_v2 have and most (but not all) are implemented yet. Some methods will never be implemented, or are complicated so will not be implemented soon, these methods are marked with the error attribute to get a readable error when used. The methods for channel configuration are also missing, but those just need a closer look and an implementation. This resolves a number of TODOs and makes uplink work using the MKRWAN Stream API, makes downlink work and implements most configuration methods (and adds a few more). It also adds documentation to most methods, though some overall documentation is still needed. The blocking/non-blocking behavior of join and data transmission is still incorrect (to be decided how this shouls work exactly).
This splits the join() method into separate joinOTAA and joinABP methods and makes joinOTAA block. This also adds a joinOTAAAsync method, to still allow non-blocking joins. joinABP is as before, since there are no background tasks there, so it just returns quickly already.
There was already a regular if for both cases, so this probably makes things easier to read.
This deviates from the MKRWAN implementation, which is only blocking for confirmed uplinks. For this library, making non-confirmed uplinks non-blocking would only work if the sketch handles calling maintain() until the RX windows are done, so sketches will need to be slighlty modified anyway. Always blocking makes the regular API more consistent and easy to use, and all blocking methods have non-blocking Async equivalents that are slightly more involved to use (because they require calling maintain()). This also adds endPacketAsync(), a non-blocking version, and exposes the send() method publically to allow sketches to pass an existing buffer directly.
This allows gcc to warn about invalid parameter types.
This allows enabling and disabling channels, but not defining new channels (MKRWAN has no API for this either, and in most cases the network will take care of channel management anyway).
The parsePacket method is now documented elsewhere.
This example is mostly kept verbatim as copied from MKRWAN, but this comment still references that board so is too confusing to keep unchanged.
Previously, it would check `modem.connected()` after the join returned, which was needed when the API was asynchronous, but now this can just check the return value and get the same result.
matthijskooijman
force-pushed
the
complete-first-version
branch
from
fda80a5
to
8d90ad8
Compare
fpistm
requested changes
Sep 23, 2022
|
I've made a script to add pragma to avoid unused parameters warning here: I guess you can ad dit and apply it. |
|
Hi @matthijskooijman |
This scripts add pragma directive to ignore unused parameter warning to STM32CubeWL files. Signed-off-by: Frederic Pillon <frederic.pillon@st.com>
These warnings are caused because frequencies are stored as `uint32_t`, which is `unsigned long`, while the printf format expects `int`. In practice, this does not actually cause problems, since on STM32 gcc `long` and `int` are both 32-bits and frequencies are never large enough to cause signed vs unsigned ambiguity. Since printf has no format specifiers for e.g. uint32_t (libc does have some macros for this, but those really hurt readability), this is tricky to fix in a portable way (other architectures or compilers might have `uint32_t` equal to `unsigned int` instead of `unsigned long`), this fix just casts the frequency to `unsigned` before passing it to printf (and for good measure, also convert the specifier from `%d` to `%u`). This does mean this printing will break if `int` is not at least 32-bits (e.g. on AVR), but given the scope of this library, that should be acceptable.
This is already a dummy operatino in the original MKRWAN, but we made it deprecated in STM32LoRaWAN, so remove the call to prevent confusion and a warning.
Cool. I've slightly reworked the script and added it, and also applied it. I've also fixed the remaining warnings in STM32CubeWL, maybe those commits can be forwarded to the lora team for upstream inclusion? Ideally the unused parameter warnings would also be fixed, but I suspect that might partly be something for upstream LoRaMac-node rather than STM32CubeWL (but I haven't checked). I've also fixed your other comments. For ease of re-review, I have used fixup commits instead of force-pushing, so this needs one more round of rebase to squash those back into the history before this PR can be merged. |
fpistm
reviewed
Oct 4, 2022
There was a problem hiding this comment.
I missed that before but there is no keywords.txt file to allows syntax coloration.
Several API are colored as they have common name (begin, write,...) even joinOTAA is colored, I guess due to other library installed. Anyway, I guess having one would be better and followed Arduino library specification.
fpistm
requested changes
Oct 4, 2022
Based on Arduino MKRWAN one. Signed-off-by: Frederic Pillon <frederic.pillon@st.com>
I've added the |
Signed-off-by: Frederic Pillon <frederic.pillon@st.com>
|
Thanks for fixing those! Looks good to me too, yay! (You did forget to squash the one fixup commit, but that's ok ;-p) |
Yes I forgot the fixup. 😅 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This version should be complete in terms of code and if it passes review, can be merged to main as far as I'm concerned (future fixes and improvements can be done on top of that).
I have not fixed the codespell issues yet, I'll add a commit for that later.