drivers/sx127x: driver rework and FSK implementation

[kYc0o]

Description

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

sx127x.c/h:

• 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).

sx127x_getset:

• 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.

sx127x_netdev:

• 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

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.

https://www.semtech.com/uploads/documents/SX1272_DS_V4.pdf
https://www.semtech.com/uploads/documents/DS_SX1276-7-8-9_W_APP_V6.pdf

[jia200x]

The whole idea makes sense to me. I will analyze case by case and write a deeper review.

Generalise or remove sx127x_lora_packet_info_t (I think it was already done in some PR?).

This was already done in #11144 . It's just for LoRa though. Do you think we could generalize that new structure or we would need a netdev_fsk_rx_info_t like?

[kYc0o]

Generalise or remove sx127x_lora_packet_info_t (I think it was already done in some PR?).

This was already done in #11144 . It's just for LoRa though. Do you think we could generalize that new structure or we would need a netdev_fsk_rx_info_t like?

Hmmm I see, actually it would be the same for FSK, so it makes sense to have it in a shared place.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

[isavitsky]

Hello,

Is there an FSK patch for this driver to try out?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

[geonnave]

Hi, any news on LoRa FSK support? Thanks in advance.