Instead of getting the status from the very first byte in the input
buffer, which will be what we read when we send the first byte. Let's
instead get the status from the first byte after the command. This
provides a more accurate status value.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
The SX126x get status command returns a single status byte. The status
byte is automatically read in the `Module::SPItransferStream()` function
when we increment buffLen (see
https://github.com/jgromes/RadioLib/blob/master/src/Module.cpp#L259).
By setting numBytes we incorrectly end up reading 2 bytes. Instead set
numBytes to zero so we only read the single status byte.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
In Module::waitForMicroseconds if RADIOLIB_INTERRUPT_TIMING is defined the variable prevTimingLen has been incorrectly upated in v6.0.0 introducing a bug that stops compilation.
Fix: renamed _prevTimingLen to prevTimingLen to match other updates
This was introduced when STM32WL support was added. Using constexpr for
the END_OF_MODE_TABLE constant allows it to be initialized in the class
declaration, but this needs C++11. This moves the initialization out of
the class declaration to the .cpp file, which does not need constexpr.
It also lets STM32WLx::END_OF_MODE_TABLE define its value directly
(instead of aliasing Module::END_OF_MODE_TABLE) to prevent reduce
runtime overhead (see below).
The downside of this change is that the value of the END_OF_MODE_TABLE
is no longer visible in other compilation units and thus cannot be
inlined into the rfswitch_table (if used).
For example, on STM32, this means that instead of having a pre-cooked
rfswitch_table that lives in the .rodata section (so can be read
directly from flash), the table lives in RAM and is initialized at
runtime (the actual modes and pins are copied from flash to RAM by the
standard startup loop that copies all of the .data section, and the
END_OF_MODE_TABLE value is copied by a bit of new generated code). This
means a little more runtime overhead, but the cost is mostly in RAM size
(80 bytes for the SMT32WL sketches, 16 per mode plus 16 for the
END_OF_MODE_TABLE).
In a first attempt at this commit, STM32WLx::END_OF_MODE_TABLE was still
initialized using the Module::END_OF_MODE_TABLE value, but since the
latter is also not available at compiletime, this meant initialization
of the former also needed to happen at runtime, adding even more code
overhead (and possibly leading to ordering issues as well). To avoid
this, the STM32WLx::END_OF_MODE_TABLE initialization now just duplicates
that of Module::END_OF_MODE_TABLE.
On AVR, the impact is not so much: Since AVR cannot address flash
directly, the table was already copied from flash to RAM at startup, so
the extra RAM usage is just 4 bytes because END_OF_MODE_TABLE is now
also present in RAM, to be copied into rfswitch_table at startup.
Options for avoiding this overhead (not implemented in this commit)
could be (in no particular order):
1. Use a macro instead of a constant. Downside is that these cannot be
scoped inside the Module/STM32WLx classes like now, so this requires
changes to sketches that use a rfswitch_table (and reduced scoping
and using macros adds more opportunity for conflicts and weird
errors).
2. Apply the change in this commit only when C++11 is not available.
Downside is that the initialization value of these constants must be
duplicated in the .h and .cpp file for C++ and older versions
respectively.
3. Let sketches just use `{Module::MODE_END_OF_TABLE, {}}` explicitly
instead of `Module::END_OF_MODE_TABLE`. Downside of this is that this
requires sketches to be modified and that it lets the sketch encode
more of the table structure, potentially making future API changes
harder (but it probably does not really matter in practice).
4. Turn END_OF_MODE_TABLE into a static method, which *can* then be
defined in the class declaration and inlined. The method can then be
conditionally marked as constexpr, which allows C++11 compilers to
completely resolve the rfswitch_table value at compiletime, producing
a binary identical to before this commit. When constexpr is omitted
(e.g. on older compilers), some runtime overhead is added (pretty
much the same as the result from this commit). Downside is that
sketches must be modified, and the `END_OF_MODE_TABLE` "constant"
must now be called, e.g. `END_OF_MODE_TABLE()` which might be a bit
unexpected syntax.
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.
jgromes