Merge Lacuna Basic MAC #11
Draft
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
This adds an implementation of the LMIC HAL, plus some files needed for the Arduino library format and some examples. In addition, an export.sh script is added that can be used to generate an Arduino library from this repository. This script only reorders files, it does not change anything. When given the --link option, it creates symlinks instead of copies, allowing to test things in the Arduino IDE easily, while committing the original files in this repository. The HAL, examples and library.properties files are taken from the https://github.com/matthijskooijman/arduino-lmic repository, revision 90bc049 (Print downlink ack status in ttn-otaa example too). Only the configuration mechanism was changed slightly.
Previously, this used GNU-specific cp options, which are not available on OSX.
This stops using the osjob scheduling from lmic for scheduling transmissions, since this is a bit tricky to integrate with existing code, and users now often think that using it is mandatory. By removing it, and replacing it by simple time-based checks for scheduling transmissions, it should be more clear how this works. Also, remove some code specific to the Pinoccio board.
Examples are untouched, those will be updated separately.
This declares the onLmicEvent() callback function that must be defined by an application. Previously, the application was expected to define this itself, and a reference to it was declared in lmic.c. By moving the declaration into lmic.h, this prevents problems when an application uses a different function signature for the function, or when defining the callback in a C++ file (since lmic.h uses extern "C", the compiler now knows to generate a C-style function rather than C++).
The AES_S array contains 16-bit integers, which are shifted upwards during processing. On platforms where int is 32-bit or wider, these integers are integer-promoted to 32-bit before shifting, making them work as expected. On smaller platforms, this does not happen, causing the upper 16 bits to be shifted off before the value is converted to a 32-bit integer. By manually casting these values to 32-bit before shifting, this issue is prevented.
These functions convert four individiual bytes from a buffer into a 32-bit integer. The upper two bytes were cast to u4_t before being shifted in place, but not the lower bytes. It seems that this was not needed, since even when int is 16-bits, shifting a byte by 8 will not cause any bits to "fall off". However, a problem with sign-extension would occur in some cases. The problematic expression is: return (u4_t)(buf[0] | (buf[1]<<8) | ((u4_t)buf[2]<<16) | ((u4_t)buf[3]<<24)); Here, `buf[1]` would be integer-promoted before shifting. It is promoted to a *signed* `int`, since the original `u1_t` type is small enough to fit in there. Now, if the MSB of `buf[1]` is 1, it is then shifted to become the MSB (i.e. sign bit) of the resulting *signed* integer. Then, before the bitwise-or operator is applied, this value is extended to 32-bits, since the right operand is 32-bit. Since the left-hand side is signed, it is sign-extended, causing all upper 16 bits to become 1, making them also 1 in the resulting value. To fix this, all bytes are first explicitely cast to `u4_t` to make sure they are already big enough and remain unsigned. For consistency, the same is done for `os_rlsbf2()`, even though this same problem cannot occur there (C guarantees that an int is at least 16-bits wide).
The original LMIC AES implementation is optimized for fast execution on 32bit processors, but it ends up taking a lot of flash space on 8-bit AVR processors. This commit adds an alternative AES implementation written by Ideetron, which is a lot smaller, but also about twice as slow as the original implementation. This new implementation is selected by default, but the original can be used by modifying config.h.
Disabling this shrinks the code by a fair bit, so this is interesting for low-memory applications that do not need class B. This can also disable processing of some MAC commands. They are completely not recognized anymore, which should be ok since these commands should never be received at all. If they are received, this will break processing of subsequent commands, though.
This allows the .h files to be included from C++ code directly, without having to worry about calling conventions and name mangling.
The `LORA_RXSTART_FIXUP` macro was not defined for this board, breaking compilation. This just defines it to 0, which is probably incorrect, but related values are also 0 and to be determined, so this at least fixes compilation.
In practice, u4_t and s4_t arguments are always passed to %F (freq mostly). On systems where int is s4_t, this works fine (the signed vs unsigned mismatch works out as long as values are smaller than 2^31). However, on systems where int is s2_t, this causes the printf arguments to run out of sync, producing garbage output and potentially infinite loops. In two cases, %F was used for a s1_t, which must now be ucast to s4_t (it would already be implicitly converted to int, so again this was not needed on systems that have int equal to s4_t).
This allows printing long values. On ARM, these are identical to int, but on AVR they are different, so this allows printing 32-bit values there. This is not actually used anywhere yet (the only 32-bit values used are timestamps, which are printed using %t), but this prepares for printing raw timestamps in a subsequent commit.
This supports a CFG_DEBUG_RAW_TIMESTAMPS define, which changes timestamps from the hh:mm:ss.mmm format to a raw tick value (which might be harder to read, but has more precisions). The default is still the millisecond format.
This uses %t for printing ostime_t values. The field width in the format string is not normally used, but added for when the "raw" ticks timestamp format is applied (with CFG_DEBUG_RAW_TIMESTAMP).
On e.g. ARM, %d and %ld are equivalent, but on smaller architectures like AVR, they are not, so this makes these prints work there.
This is triggered by a CFG_DEBUG_VERBOSE define and gives more spammy debug output. This also adds some sleeping-related debug output, but this is disabled for now since the Arduino port does not do sleeping yet. The osxtime_t values (which are 64-bit) are printed as ostime_t (which are 32-bit). Printing them as 64-bit would make it harder to compare them with ostime_t values. Things might get weird or incorrect after an overflow, though.
This allows the HAL to indicate reset is not connected, which causes the reset to be skipped entirely. This will often work as expected, but (especially when the MCU is reset while the transceiver is working) might cause problems, of course.
This allows seeing startup messages and errors on native USB boards.
This was already used for the abp example, now they are consistent again.
This should probably be made configurable, but for now just always enable it (since typically DIO3 is not used as anything else anyway). The voltage and timeout values were taken from other example code and should probably also be configurable. Also, the timout adds TX/RX delay, so should probably be accounted for.
This actually breaks the build in recent gcc versions.
The SX126x chips can map all interrupts to DIO1, so no need to connect anything else.
This prevents USB operation on native USB boards, which complicates subsequent uploads.
We should really use them incrementally and save them, but for now the LoRaWAN 1.0 behaviour of random nonces should work.
On some Arduino cores, printing with IRQs disabled is problematic (see e.g. arduino/ArduinoCore-samd#472). In general, disabling interrupts for shorter amounts of time is a good idea anyway.
This transceiver is getting more common and one of the primary reasons for using basicmac, so make it the default.
Configuring the TCXO is not possible using a MCU-controlled GPIO on the SX126x, so this function makes no sense there. This prevents accidentally calling it anyway.
Enabling this unconditionally originally seemed harmless, since the pin was otherwise unused, but this also switches the transceiver into TCXO mode, breaking operation with a crystal. Instead of enabling this unconditionally, it is now disabled by default and can be enabled explicitly. On Arduino, set the `tcxo` field of the pin map to `LMIC_CONTROLLED_BY_DIO3`. With Makefile-based stm32, define `LMIC_DIO3_CONTROLS_TCXO` on the compiler commandline. The API between the radio driver and the HAL is not ideal and might need to be reconsidered later, but since the change on the Arduino side is breaking, better to get that out of the way soon (and the HAL side can be improved later). This fixes #9
With the added `LMIC_CONTROLLED_BY_DIO3` pin value, the pin value asserts need to be slightly changed (and some asserts added for optional pins) to make sure that this new value is not considered a valid value for all pins. This also moves some asserts around to form a single block per transceiver type, rather than being spread around in multiple `#if` blocks.
There was code to run without a busy pin, but the initialization asserts would forbid setting it to LMIC_UNUSED_PIN. This should make that work again.
Enabling this unconditionally originally seemed harmless, since the pin was otherwise unused, but there is a (tiny) chance that boards have unused DIO pins grounded, which could lead to short-circuits. Also, it is probably good to make the DIO2-controls-TXRX situation explicit in the pinmap and be consistent with DIO3-controls-TCXO. Instead of enabling this unconditionally, it is now disabled by default and can be enabled explicitly. On Arduino, set the `tx` field of the pin map to `LMIC_CONTROLLED_BY_DIO2`. With Makefile-based stm32, define `LMIC_DIO2_CONTROLS_RXTX` on the compiler commandline. The API between the radio driver and the HAL is not ideal and might need to be reconsidered later, but since the change on the Arduino side is breaking, better to get that out of the way soon (and the HAL side can be improved later).
SX126x: Make using DIO2/DIO3 to control TXRX/TCXO optional
Previously, target-config only showed the SX1262, not the SX1261. This fixes that by adding a commented define. It also completes the guard around the entire block, which is relevant for automatic (Github Action) builds (before this change both `BRD_sx1261_radio` and `BRD_sx1262_radio` might end up defined, which did not cause a compilation failure, so it was not noticed before).
This removes the hardcoded EU868 region from examples by just querying BasicMac for the first enabled region instead (thanks to Luiz Henrique Cassettari for suggesting this in #9). Additionally, this puts examples of all regional defines in target-config.h and documents that multiple can be defined at the same time (though multiple regions and other regions than EU868 have not been tested yet, see #8).
This comment was updated in the pinmap of basicmac-abp, but forgotten to be copied to basicmac-otaa.
This is a band that was somewhat recently made available and can be used for LoRaWAN. The max allowed bandwidth is 350kHz (According to ERC recommendation 70-03), but since BasicMac has no way to encode this limitation currently, this is not enforced (so this should be taken into account when configuring channels instead).
TTGO T-Beam pinmap
This was all commented out and unused, so just remove it. The file must still exist because it is included from elsewhere, though this can probably be changed in the future as well.
Some parts of this code were still marked with the EPL license, used by older versions of LMIC. This changes the license to 3-clause BSD, the same as the rest of LMIC. This license is also added to the files that previously did not state any license. All of the affected code was written by me, with no significant contributions from others.
This pointed to the wrong commit, and it turns out github does not autolink commits in the README, so also make this an explicit link.
When using DIO2 or DIO3 to control the TCXO or RxTx switch and GetRandom is called while the radio sleeps, the random generator will not be able to receive noise and return only zeroes. This fixes this by setting up the settings whenever GetRandom is called. To make sure CommonSetup is visible, also move GetRandom a bit downward in the file.
This radio has a random generator built in, so use it to seed the internal random generator. For the sx127x, the old, imperfect, method (based on DevEUI and compilation time is still used. This could maybe use RSSI measurements like LMIC did? Previously, the random generator was seeded on first use, but to prevent needing the radio while it is already operating, this now seeds the generator on startup. In particular, this means that on sx126x, the channel selection and join nonces are now properly randomized.
This previously used a customized version, but all customizations have been merged upstream, so use the standard version again.
* origin/master: (84 commits) direct PIO API (lower overhead to aid in implementing bit-banged protocols) timer peripheral timer peripheral -wip- timer peripheral beginnings spell check perso spec add missing DMA channel definition for USART2 persotool, move perso uart back symlink perso test perso: cleanup, test cases perso -wip- unicorn: implement reset, perso -wip- perso -wip- simul peripheral fixes, perso -wip- GPIO simulation, USART simulation fix perso mode -wip- perso mode -wip- add idle detection to UART API, auto-pull and active high/low enhancements in PIO API increase test case robustness Update format of license so GitHub can auto-detect the type first draft of perso spec first draft of perso spec first draft of perso spec perso/test mode -wip- uart simulation -wip- change appstart hook signature to support multiple startup options svtool: support hook priorities use absolute URL for basicloader submodule updated readme and getting started guide cleanup multi-instance uart -wip- don't allow baud rates > 10M on LPUART multi-instance uart -wip- multi-instance uart -wip- multi-instance uart -wip- adressing comments from PR #4 review ex-join app test pass hex files for test in environment, symlink test from project cleanup: remove dockerfile, remove pylora (now using liblora on PyPI) add numpy to requirements.txt replace old simulation with new, add requirements.txt, re-activate test target in makefile, add test to travis ci build travis ci: install basicloader requirements, updated basicloader ref new travis ci file certification tests complete, traffic trace simul rework: even more compliance tests typing fixes simul rework: more compliance tests simul rework, more compliance tests simul rework: more compliance tests typing fixes simul rework: starting to port the compliance tests ...
|
Quick notes on the failing Unicorn tests:
|
|
Thanks for picking this up already :-)
You mean "enable" here, I think? Interrupts are enabled because a lot of Arduino platforms will not manage serial output with IRQs enabled (AVR will manage if you call One thought I had is that maybe
Hm. I changed it to use the SX126x random generator when possible, since the Arduino API has no standard way to get a proper random value (just pseudorandom that needs the sketch to seed it with some real entropy, which often users don't do). So I removed the lazy initialization, to ensure random is initialized after the radio is initialized. Can we somehow shuffle initialization order so that eefs is initialized after that? |
|
@mkuyper, can we maybe move this merge forward? Is there anything you need from my side? |
|
Hey @matthijskooijman .. thanks for bringing this back to the top of my stack. I have some work I've been doing on the nRF52 port that I want to merge in and then I'll get back to this PR. |
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.
The intention of this PR is to serve as a starting point for merging the Lacuna version of Basic MAC into this repository. (Issue #9)
The current state is just a raw merge with conflicts rudimentarily addressed. Note that while the ex-join example appears to build, it does not currently pass any tests.