[riot-notifications] [RIOT-OS/RIOT] drivers/sx127x: driver rework and FSK implementation (#11333)
notifications at github.com
Tue Apr 2 19:58:17 CEST 2019
<!-- Please describe your use case, why you need this feature and why this
feature is important for RIOT. -->
While implementing the FSK mode for sx127x devices I faced several problems which led me to question the current driver API and structure.
I'm aware the driver was implemented specifically to fit for the LoRa needs, which didn't make it super flexible to use the rest of its features.
Thus, I'd like to propose a plan to rework the driver in order to add the FSK mode in a more transparently way.
My suggestions/questions are made regarding the current status of the driver, minimizing the efforts and API changes
- Change `sx127x_init()` into `sx127x_lora_init()` and excecute it if LoRa is selected as the default init mode. This might need a variable to define which is the default init mode.
- Move the common settings out from `sx127x_lora_settings_t lora` into `sx127x_radio_settings_t` (e.g. `preamble_len`, `power`, `bandwidth`, `lna`).
- Define demodulator specific values outside the generic header to lora.h (e.g. frequency, power, buffer size, etc).
- Avoid to set DIO registers on modem mode set function.
- Should all register options made available as get/set? There are lots and this means having at least a `uint8_t` variable per register and several flag bits. However, most of the registers are used if switching between FSK/LoRa modes, thus a fine grained access to them becomes super useful. This might be limited to a selection of them but in my experience almost all are set/used.
- set rx/tx mode: allow to give specific DIO init settings, or factor out these settings. This is needed because the DIOs can be configured in very different modes according to the physical wiring.
- Increase the size of `payload_len` variable (`uint8_t` -> `uint16_t`) on the setter for this register, since for FSK can be much more than 256B.
- Should LoRa specific setting be namespaced (`sx127x_lora_setting()`)? I think this would ease the usage of the upcoming FSK functions, which would also be namespaced. Actually another (namespaced) file for them can also be an option.
- Generalise or remove `sx127x_lora_packet_info_t` (I think it was already done in some PR?).
- `_init` function takes no parameters currently, which doesn't allow to choose between one mode or another. However, this might not be completely needed if a global define is used to choose the default radio mode.
- Add all DIOs callbacks by default, eventually with dummy handlers which can be overridden. Currently there are only 3 handlers because of LoRa but it doesn't depend on the mode but on the wiring of the board embedding the device.
- Make the callbacks for the DIOs `extern` so the user can define them according to the wiring of the DIOs.
IMO most of the changes are related to the settings (register set/get) and the way DIOs are configured/accessed, so we can concentrate more on that.
### Useful links
<!-- Please include links to any documentation that you think is useful. -->
Datasheets provide tables for all the registers and a nice overview of the shared registers in both FSK and LoRa modes. Also, a table describing the DIO settings is also provided, so we can see all the possibilities and why it needs to be highly configurable.
<!-- Thanks for contributing! -->
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the notifications