embassy-lora Sx126xRadio pin types

[jbeaurivage]

I'm currently trying to use Sx126xRadio that #985 has brought. As defined here, it seems that there is only one type parameter for all output pins, and one type parameter for all interrupt-input pins:

impl<SPI, CTRL, WAIT, BUS> Sx126xRadio<SPI, CTRL, WAIT, BUS>
where
    SPI: SpiBus<u8, Error = BUS> + 'static,
    CTRL: OutputPin + 'static,
    WAIT: Wait + 'static,
    BUS: Error + Format + 'static,
{
    pub async fn new(
        spi: SPI,
        cs: CTRL,
        reset: CTRL,
        antenna_rx: CTRL,
        antenna_tx: CTRL,
        dio1: WAIT,
        busy: WAIT,
        enable_public_network: bool,
    ) -> Result<Self, RadioError<BUS>> {
        let mut lora = LoRa::new(spi, cs, reset, antenna_rx, antenna_tx, dio1, busy);
        lora.init().await?;
        lora.set_lora_modem(enable_public_network).await?;
        Ok(Self { lora })
    }
}

@lulf and @ceekdee, since you've worked on that PR: Unless I'm mistaken, does this mean that all the CTRL pins would need to have the same type, and all the WAIT pins likewise? Would it make more sense to have a unique parameter for each of these pins?

[lulf]

Both the nRF and STM32 HALs provide an AnyPin type you can create, i.e. using p.PB4.downgrade(). In general, I think we want to have a RadioSwitch parameter for the antenna pins at least, because it can differ from board to board. I think @ceekdee is planning to fix that.

[jbeaurivage]

Ah, fair enough. I want to use it with atsamd-hal, but it has a similar DynPin API.

Regarding the antenna pins, I believe my board has a single switch for both TX and RX. How is it recommended to handle that case?

[lulf]

Until the API has been refactored, I think you maybe need to wrap your pin in some custom shared pin type containing an inner type with a RefCell that the driver can use. Since TX and RX is not done at the same time, it should be 'safe' to do this.

It's exactly the reason I think we should use a RadioSwitch trait similar to the other drivers, because boards do it so differently.

[ceekdee]

In the original implementation, each pin did have its own type. This seemed cumbersome and unnecessary to me, and when I realized that AnyPin could be used for both input and output pins, I changed to use CTRL and WAIT. This can certainly be re-visited as we attempt to combine MCU/Semtech chip combinations into one implementation (#1023). Any additional observations and guidance you wish to provide can be placed in issue 1023.

Regarding the antenna processing, another option until the re-implementation is to make local changes to the following functions in board_specific.rs in the sx126x implementation:

• brd_ant_sleep()
• brd_ant_set_rx()
• brd_ant_set_tx()

to support your use case.

[lulf]

@ceekdee I might have a go at introducing a RadioSwitch trait for the sx126x in a way that should help this use case, if that's ok with you.

[jbeaurivage]

Would it make sense to extend the RadioSwitch trait to all board-specific functionality, which the user is expected to implement? All of the generic complexity could be hidden behind it so that the radio structs wouldn't have to deal with individual pins. A default struct which implements that trait could be provided, for users that don't have specific needs or can provide type-erased pins. I can also try to whip up a POC if y'all are interested.

[lulf]

By other board-specific functionality, do you mean the Wait, Ready etc. pins? If so, I guess that could be there as well, though I've not seen them having different semantics across boards, vs. the radio pins which seem to vary a lot more.

I'm fine either way, I think it's worth exploring.

[ceekdee]

A brief synopsis of architectural thoughts to this point for the implementation in issue 1023. A bit premature, but I will be unavailable to work on an implementation until the end of November and want to encourage other thoughts.

• produce a trait that describes LoRa API functions that are prevalent across Semtech chips (the base API, largely present in the sx126x implementation - https://github.com/embassy-rs/embassy/blob/master/embassy-lora/src/sx126x/sx126x_lora/mod.rs - which closely followed the Semtech C implementation);
• produce additional traits for LoRa API functions unique to different flavors of Semtech chips;
• implement enums and structs which support the various concerns represented across the full API, making each as generic as possible to support present and future LoRa chips (this will build on the separation of concerns in the stm32wl driver);
• implement an orchestrator for each MCU/LoRa chip combination. For example, the rak4631 will have an orchestrator, as will a STM32 nucleo board;
• each orchestrator is a lightweight implementation which grabs the device-specific pins and SPI bus and assembles the necessary enums and structures to support the MCU/LoRa chip combination;
• the LoRa generic and board-specific API for an MCU/LoRa chip combination will be accessed through the orchestrator.

[lulf]

I've moved the discussion over to #1023 now, focusing on this issue here:

Would it make sense to extend the RadioSwitch trait to all board-specific functionality, which the user is expected to implement? All of the generic complexity could be hidden behind it so that the radio structs wouldn't have to deal with individual pins. A default struct which implements that trait could be provided, for users that don't have specific needs or can provide type-erased pins. I can also try to whip up a POC if y'all are interested.

I think that makes sense, maybe have a look at the stm32wl integration in embassy-lora for inspiration and see what you think.

[jbeaurivage]

@ceekdee, I've gotten started a rough API that abstracts over the physical aspects of a board (view here). However while trying it out, I seem to consistently run into a bug in this function where indexing buffer overflows when i reaches 4. Seems like whe debugging, number_of_registers is set to 34, is this normal? The SX126x datasheet is extremely sparse when it comes to register retention stuff, so I can't really effectively debug.

Edit: Nevermind, the bug lied within my SPI driver.

[ceekdee]

The original sx126x driver has been removed in favor of using the lora-phy crate. Therefore, closing this issue.