add shared modulation types for gfsk, lora

[ryankurte]

playing with common channel definitions for GFSK and LoRA, as a step towards a more cross-radio consistent channel interface. it's only a start because configuring modulation takes way more than just the channels, but any approach we take is going to need these shared definitions anyway.

also updated the preliminary / barely-used Config trait to use an associated type bound instead of a wrapper error type.

(it appears tests are broken because we've released another [email protected] but, i don't have it in me to update everything rn)

cc. @henrikssn

[henrikssn]

Nice work, I like the direction this is going in.

What about sync word? I think that is essential for implementing a generic channel.

[henrikssn]

This needs to be hertz, at least for the sx1231 most RxBw options are not aligned with kHz.

[henrikssn]

Also, is this receiver bandwidth or transmitter frequency deviation?

[ryankurte]

good question, i was aiming for modulation bandwidth (ie. frequency deviation) as ime this is how channels are usually described / receiver bandwidth is typically driven by this

[henrikssn]

Can't we make this Hz? It would top out at 4.2GHz, but is that going to limit any use case we have in mind?

[ryankurte]

the decawave gear (DWM1000 etc.) goes up to 6.5GHz so, i think we're better to keep it wider. do you see a need for sub-khz definitions? because if so we could have a more complex Frequency type

[ryankurte]

(and ngl i would love to have a proper parser that understood frequency units)

[henrikssn]

I don't have a concrete use case, but this feels like a requirement which is not going to be true for all implementations.

What about using embedded time Rate with concrete type being an associated type which defaults to kHz?

https://docs.rs/embedded-time/0.12.1/embedded_time/rate/trait.Rate.html

[ryankurte]

at the moment we're avoiding embedded_time in e-h due to the complexity, so, i think i'd prefer a simpler type defined by us for now.

[henrikssn]

Makes sense, we can define our own Frequency trait and implement that for e-t Rate (behind a feature). Then users can decide whether they bring their own type or use e-t.

RTIC uses a similar approach for their Monotonic: https://github.com/rtic-rs/rtic-monotonic/blob/4050b236f36ef99da2d88a9e2d4a211a8d46d262/src/lib.rs#L34

[ryankurte]

What about sync word? I think that is essential for implementing a generic channel.

so this is where things get a bit more tricky, at the moment i have separation between the modulation and the channel configurations, with the aim of splitting what you need for channelisation from the wider radio configuration.

ime it's much less regular to be switching things like sync-words / preambles / whitening at runtime (tho for swapping modulations you have to do this anyway, understanding my use case is not the most common), and the modulation configuration is much less consistent between devices, so it's nice to isolate that from swapping channels. i think the reality is you're always going to have some device-specific configuration so similar to e-h we've gotta aim for operation more than configuration.

i've been playing with the generic channel configuration some and, all the different approaches have some solid limitations, but i'm thinking for actual interoperability we might be better to add channel functions for each protocol rather than modulation, so alongside basics like Channel<LoRa> (which is already pretty close to workable) we might have Channel<BLE> or Channel<Ieee802154g> which does all the correct per-radio configuration for a well-defined BLE or 802.15.4g defined channel.

[henrikssn]

I think it makes sense to split Modulation and Channel concepts. Channel is nice to have for e.g. implementing channel hopping in a generic way. Modulation can be implemented (or not) by the radio if it supports the relevant modulation. Preamble, sync word, packet length, whitening, crc, etc would then be part of the modulation.

One thing - should Channel be a trait too? I suspect at least frequency and bit rate to be shared across different channel types (e.g. for LoRa, bitrate equals bandwidth IIRC). It might also be a good idea to use getters instead of public fields to facilitate trait generalization later.

[Tortoaster]

Would this also be the place to add types for the associated types of the other traits, such as Info and State?

#[derive(Debug, Default)]
pub struct LoRaInfo {
    rssi: i16,
    snr: i8,
}

(I'm not sure what LoRaState would look like.)

[ryankurte]

Would this also be the place to add types for the associated types of the other traits, such as Info and State?

like RecieveInfo and RadioState? (which are definitely not complete just yet)

from #12:

Currently, my WIP implementation of the radio traits for the SX126x in stm32-rs/stm32wlxx-hal#248 blocks other crates that don't use LoRaChannel (from #24), but with multiple different Channel implementations, they can all coexist.

awesome! the sx126x has been on my list for a while now, would you consider implementing it as a separate crate for folks with the external SPI attached version?

at the moment drivers mostly implement a wrapper enum for the different modulations and channels they support, which gets bound in as the associated type. i found this to be the most convenient approach because you need these anyway to easily store the channel / modulation in the object without mutating type-states.

the (far from perfect) absolute minimum from here for consumers to support different radios would be to add a Channel: TryFrom<LoRaChannel> bound, which doesn't require anything else of the drivers. i'm not super opposed to moving to generic channels, but, having had a go at a few different implementations using them i am not yet sure they're the most useful abstraction.

This brings one problem, namely that implementing LoRaChannel does not guarantee that the radio is currently set up for LoRa, so you might have to set (part of) the channel redundantly from time to time.

yep, i tend to store the current modulation and return an error if the channel type and modulation do not match. on the assumption that changing channels is a far more regular use case than changing modulations, i think it's reasonable to prioritize the former for now.

for the latter, that's where the discussion about protocols come in, i wonder if it's more useful to provide a higher-level API than Channel that lets you say, configure the radio for LoRaWAN on channel 3, or 802.15.4g on channel 2, or Ble5.0 on channel 17, so you ask for the protocol and the driver takes care of all the configuration to get you there.

This could be solved with type-state, at least partially (remembering being set up for LoRa is easy, remembering for which frequency is not), but I can see how that would be a pain.

yeah, i've done a bunch of exploratory work around type-state for this and found it to be way more of a burden than a boon unfortunately. it seems like it should be super nice, but as soon as you end up moving / mutating the radio objects you just end up pushing a bunch of complexity on the driver users.

[Tortoaster]

like RecieveInfo and RadioState? (which are definitely not complete just yet)

Sort of, I mean concrete types that implement those traits. Currently, the implementation in stm32-rs/stm32wlxx-hal#248 depends on https://github.com/Tortoaster/lorawan-device for the LoRaInfo type (alternatively, https://github.com/ivajloip/rust-lorawan has the same type with a different name), because it needs a concrete Info type to implement the Receive trait.

I'd like to move this type (and possibly others, such as LoRaState, although I'm not sure how that would look) to radio-hal, because then the lorawan-device crate would work for any radio that implements the radio-hal traits, as long as they implemented the traits using those dedicated associated types.

If they have to use the lorawan-device types, it defeats the purpose of the traits, as they could only be used by lorawan-device. On the other hand, if they use their own types instead, the lorawan-device crate cannot use them generically. So that's why I thought they should be included here. Please correct me if I'm missing something!

at the moment drivers mostly implement a wrapper enum for the different modulations and channels they support, which gets bound in as the associated type. i found this to be the most convenient approach because you need these anyway to easily store the channel / modulation in the object without mutating type-states.

Makes sense! I will change my channel implementation to use an enum wrapper as well. If I understand it correctly, the same principle applies to Info as well, so an enum wrapper for LoRaInfo, GfskInfo, etc. would also be useful, right? In that case, it would be useful for my use case if radio-hal included all those enum wrappers and their inner types. However, that would require, among others, a wrapper with Info variants for all different possible modulation types of any radio, which is not very pretty. 😁

This does seem quite limiting though. If generic type parameters for the traits and monstrous wrapper enums are off the table, every radio needs their own types (or at least there will be many different possible associated types to choose from). How would one use any LoRa-capable radio generically?

awesome! the sx126x has been on my list for a while now, would you consider implementing it as a separate crate for folks with the external SPI attached version?

I didn't implement the sx126x. Unfortunately, I am not quite skilled enough at embedded software to understand it. Maybe @newAM can answer the question for you!

yep, i tend to store the current modulation and return an error if the channel type and modulation do not match. on the assumption that changing channels is a far more regular use case than changing modulations, i think it's reasonable to prioritize the former for now.

for the latter, that's where the discussion about protocols come in, i wonder if it's more useful to provide a higher-level API than Channel that lets you say, configure the radio for LoRaWAN on channel 3, or 802.15.4g on channel 2, or Ble5.0 on channel 17, so you ask for the protocol and the driver takes care of all the configuration to get you there.

This sounds very intuitive, and also largely mitigates the downside of having to switch channels redundantly. My current implementation of Channel changes both modulation and channels, the former being redundant in most cases. Splitting those sounds like a good idea. 🙂

[newAM]

awesome! the sx126x has been on my list for a while now, would you consider implementing it as a separate crate for folks with the external SPI attached version?

I didn't implement the sx126x. Unfortunately, I am not quite skilled enough at embedded software to understand it. Maybe @newAM can answer the question for you!

This should be possible.

A few people using the STM32WL have told me the integrated radio is slightly different than the sx126x, but I need to buy some SX126x's and check that myself. The SPI commands/registers should be compatible for both, but with different side effects in some cases. The HSE/TXCO clock is tightly integrated with the system in the STM32WL, and entering low power modes from the CPU can automatically put the radio into sleep mode. There are also a few differences in the ST reference manual vs the SX126x datasheet, but ST is working to reconcile those.

Do you know of a source for SX126x modules that are not the integrated STM32WL? The only ones available on digikey are >$500 which is not within my hobby budget.

[ryankurte]

TLDR: happy to have common types defined here. i'm using BasicInfo and ReceiveInfo on LoRa capable devices, not sure i see a need for enum versions of this. we're still working on exactly how we're going to abstract modulation configuration, needs exploration and demonstration, but it would be useful to separate that from this PR to add a set of shared types. for now (once this has landed) either bound with TryFrom<LoRaInfo> or perhaps provide a trait with associated const channels for each radio type.

---

Sort of, I mean concrete types that implement those traits. ... because it needs a concrete Info type to implement the Receive trait.

ahh like BasicInfo then ^_^

If I understand it correctly, the same principle applies to Info as well, so an enum wrapper for LoRaInfo, GfskInfo, etc. would also be useful, right?

mostly for RX the info is RSSI and maybe an LQI, it doesn't substantially vary with modulation. i've tended to just use BasicInfo and set the lqi value to zero when unavailable, but it could equally be an optional field and if you think this missing something for LoRa use we can either add that as a field or a LoRaInfo type.

In that case, it would be useful for my use case if radio-hal included all those enum wrappers and their inner types.

If generic type parameters for the traits and monstrous wrapper enums are off the table, every radio needs their own types

every radio does need their own types, because every radio device requires different configuration. if we only exposed a common subset of options you would only be able to implement a common subset of functionality.

i think the best approach at the moment is to implement your own enums based on what the radio can support / the configuration it requires, and to implement something like TryFrom for the generic versions on the concrete types (understanding we'll aim to provide another configuration API in radio similar to this when we've worked it out).

How would one use any LoRa-capable radio generically?

in e-h as well as here there is a separation between configuration and use, you're always going to have to do some device specific configuration.

if you implemented TryFrom for your channel type (something like this) you could add a bound something like <T as Channel>::Channel: TryFrom<LoRaChannel>.

another immediate option would be to provide an extension trait with a const channel map for supported devices, which isn't perfectly abstract but, would let you plug in different radios.

it's definitely one of the goals of this project, but we are still working out exactly how to approach this here and it's going to take some effort / experimentation to get it just right, so hopefully y'all can bear with us and there are some workable alternatives in the shorter term ^_^

welcome @newAM!

A few people using the STM32WL have told me the integrated radio is slightly different than the sx126x, but I need to buy some SX126x's and check that myself.

ahh, classic but not hugely unusual

Do you know of a source for SX126x modules that are not the integrated STM32WL? The only ones available on digikey are >$500 which is not within my hobby budget.

i haven't taken any of the sx126x ones out of the packet yet but, aliexpress have quite a range of cheap radio modules and i have had good experiences with the sx127x and sx128x modules from there.

[henrikssn]

I'm very excited to see so much interest here🥳

Thinking about it more, I do feel that the channels probably should be traits, since that means we can implement them as empty structs to make this overhead free. Something like:

trait GfskChannel {
  fn freq() -> Freq;
}

struct Channel1 {}

impl GfskChannel for Channel1 {
  fn freq() {
    868.MHz()
  }
}

If you wanted to have a RuntimeConfigurableChannel, then nothing prevents you from creating one and making it implement GfskChannel as well.

Do you know of a source for SX126x modules that are not the integrated STM32WL? The only ones available on digikey are >$500 which is not within my hobby budget.

The Ebyte modules from aliexpress are good.

[newAM]

Excellent, I bought a few of the ebyte SX1262 modules to compare with the STM32WL. I do not expect to get them until 2022 (the post is very slow this time of year), if they are similar enough I will move the radio portion of the STM32WL HAL into a separate crate that works for both the integrated & discrete modules.

[Tortoaster]

mostly for RX the info is RSSI and maybe an LQI, it doesn't substantially vary with modulation. i've tended to just use BasicInfo and set the lqi value to zero when unavailable, but it could equally be an optional field and if you think this missing something for LoRa use we can either add that as a field or a LoRaInfo type.

Ah, clear! I believe LoRaWAN uses SNR as well, but I don't think it's that relevant for the device.

every radio does need their own types, because every radio device requires different configuration. if we only exposed a common subset of options you would only be able to implement a common subset of functionality.

i think the best approach at the moment is to implement your own enums based on what the radio can support / the configuration it requires, and to implement something like TryFrom for the generic versions on the concrete types (understanding we'll aim to provide another configuration API in radio similar to this when we've worked it out).

Just to make sure I understand things correctly: say I have some radio, which supports modulation types LoRa and Gfsk. I would make an enum wrapper like this:

pub enum Channel {
    LoRa(LoRaChannel),
    Gfsk(GfskChannel),
}

LoRaChannel and GfskChannel are not device-specific, but included here in radio-hal, right? Then I would implement the Channel trait for my radio, using this as its associated type.

Next, I would implement From as follows (this might be a mistake, as TryFrom is not necessary here):

impl From<LoRaChannel> for Channel {
    fn from(value: LoRaChannel) -> Self {
        Channel::LoRa(value)
    }
}

Following the same pattern for GfskChannel. This allows me to use any radio generically, as long as their device-specific Channel implements From<LoRaChannel>:

impl<T, C, E> LoRaRadio for T
    where T: Channel<Channel=C, Error=E>,
          C: From<LoRaChannel>,
          E: Debug
{
    fn lorawan_transmit(/* ... */) {
        // ...
        let channel: LoRaChannel = rate.rx1();
        self.set_channel(&channel.into())?;
        // ...
    }
}

Is this what you have in mind? I didn't realize the From<LoRaChannel> bound could be used here, thank you for the explanation!

This does require a bit of boilerplate for radios that support many kinds of modulation, with all the From or TryFrom implementations, and their implementation of the Channel trait would also be quite large. Both of these drawbacks could be mitigated by turning the Channel associated type into a generic type parameter. The Channel implementations could then be split into Channel<LoRaChannel> and Channel<GfskChannel>, and one could use where T: Channel<LoRaChannel, Error=E> to use them generically, removing the need for a From implementation, and for a wrapper enum in general. But now that I understand the work-around, it's good either way.

[Tortoaster]

Is #[non_exhaustive] necessary here and above CodingRate?

[ryankurte]

it means that adding additional spreading factors / coding rates in the future (what if we missed one!) would not be a breaking change and forces consumers to handle the unsupported case, i think it's worth it.

[ryankurte]

@Tortoaster i think you're pretty much on the same track, do bear in mind we're going to need to prototype and experiment with these interfaces for some time yet i suspect, call this PR a step forward but not a solution ^_^

LoRaChannel and GfskChannel are not device-specific, but included here in radio-hal, right?

at the moment they're device specific, but the idea of this PR is we could share em. either way doesn't matter too much for the moment.

Next, I would implement From as follows (this might be a mistake, as TryFrom is not necessary here):

the reason for a try on the conversion is that not every radio supports every channel configuration, required if we use the set_channel<T: TryInto<Channel>> bound. whether or not this is the best way to represent it, idk.

Is this what you have in mind? I didn't realize the From bound could be used here, thank you for the explanation!

not exactly, i am still aiming for the traits to be orthogonal, so you'll have the ability to setup the radio in BLE or LoRa or whatever mode, and to transmit, and to receive, and each of these should only use the bounds / restrictions it needs.

This does require a bit of boilerplate for radios that support many kinds of modulation, with all the From or TryFrom implementations, and their implementation of the Channel trait would also be quite large. Both of these drawbacks could be mitigated by turning the Channel associated type into a generic type parameter. [...]

yeah it's not perfect, i've been playing with wiring different radios into my network to get a feel for what works best, but doing this has has been pushed off my stack for the moment sorry. will get to those small changes when i can and i think the larger "how to channel" bit can happen separately.

[Tortoaster]

the reason for a try on the conversion is that not every radio supports every channel configuration, required if we use the set_channel<T: TryInto<Channel>> bound. whether or not this is the best way to represent it, idk.

I think I'm misunderstanding. Should this trait bound also allow types of configuration that could never be turned successfully into Channel at all, or does it mean that converting the supplied instance to a Channel might fail?

It doesn't sound like the former, because an Into<Channel> trait bound implies that the radio supports that channel configuration, so a TryInto<Channel> would still imply that it supports that channel configuration, but the conversion to it might fail. If configurations that aren't supported should still pass the trait bound, they would each need TryFrom implemenations that always fail.

But it doesn't sound like the latter either, since it's just conversion to a wrapper-enum, which can never fail. It might be that the radio doesn't support the supplied instance of configuration specifically, but then that error would be returned by set_channel, rather than the conversion.

yeah it's not perfect, i've been playing with wiring different radios into my network to get a feel for what works best, but doing this has has been pushed off my stack for the moment sorry. will get to those small changes when i can and i think the larger "how to channel" bit can happen separately.

Not a problem! This crate will be the compatibility layer for every radio, it would be bad to rush things 😋

[ryankurte]

It doesn't sound like the former, because an Into trait bound implies that the radio supports that channel configuration, so a TryInto would still imply that it supports that channel configuration, but the conversion to it might fail. If configurations that aren't supported should still pass the trait bound, they would each need TryFrom implemenations that always fail.

huh, yeah fair point!

Not a problem! This crate will be the compatibility layer for every radio, it would be bad to rush things

hopefully! at the moment it's still very early days though, plenty of scope for experimentation and breakage.

@henrikssn added a frequency type to play with, i recon as it stands these are useful enough to get started with

[henrikssn]

@henrikssn added a frequency type to play with, i recon as it stands these are useful enough to get started with

Awesome! When this is getting closer to merge, I would recommend making this an associated type with a constraint on Frequency trait, so that folks can bring their own Freq impl.