Add generic parameters to Transmit and Receive

[Tortoaster]

This pull request adds generic parameters to the Transmit and Receive traits. It would allow the same radio to, for instance, implement both Transmit<LoRa> and Transmit<Fsk>, each with different types of parameters for transmitting. The issue ivajloip/rust-lorawan#50 describes a use case for it. It is an early work in progress, but please let me know whether you think this is something worth pursuing! :slightly_smiling_face:

[ryankurte]

hi @Tortoaster, thanks for the PR! rust-lorawan is exactly the sort of thing i had hoped to support with radio-hal, and it's certainly up for additions and improvements to better suit this sort of use. i'll respond to this PR here, then your other comments in the linked issue.

it'd be great to have a sort generic way of specifying configuration from different modulation modes, though i'm not totally convinced associated types on tx/rx are the best way to represent this, as it'd make writing an abstraction that didn't care about modulation more tricky (for this reason i have moved away from using type-states for modulation modes) and with experience from embedded-hal i can't understate the value of minimizing trait complexity where possible ^_^

perhaps an alternative would be to add a type parameter to a configure function (or the channel function), so that is the only point at which one must consider the modulation type? alternately the approach you've taken seems like it could be represented at the type level rather than with arguments?

it's very possible i'm missing something so, happy to discuss here or organise a call if you'd like to talk about it.

[henrikssn]

I think this is a good idea, but I +1 that it would be better to handle this at the pure type level (so there is no runtime overhead of it).

Something like:

trait Transmit<P> {
  // ...
}

and then:

struct LoraRadio { ... }

impl Transmit<Fsk> for LoraRadio { 
  fn start_transmit(&mut self, data: &[u8]) -> Result<(), Self::Error> { ... }
  fn check_transmit(&mut self) -> Result<bool, Self::Error> { ... }
}

impl Transmit<LoRa> for LoraRadio { 
  fn start_transmit(&mut self, data: &[u8]) -> Result<(), Self::Error> { ... }
  fn check_transmit(&mut self) -> Result<bool, Self::Error> { ... }
}

[ryankurte]

I think this is a good idea, but I +1 that it would be better to handle this at the pure type level (so there is no runtime overhead of it).

this approach makes these traits much less usable for some of my applications as it precludes genericising over modulation types / means you have to wrap everything by modulation, and it's totally normal to run more than one configuration in an application.

as an example, i have applications testing RF links that cycle through radios, modulations, and channel configurations. at the moment i can configure a given radio then pass this into a function that takes <R: Transmit + Receive> and executes the link test, by separating the set_channel from Transmit and Receive the components that use the radio don't need to know how to configure it. we don't have any useful way to be generic over set_channel, which would definitely be useful to address!

type-state can be great for representing safety, but the overheads involved when you then have to store and mutate objects often mean it can undermine the utility of your traits. in the early radio-hal days i experimented with this to mutate the radio based on the configuration / modulation, but in practice this just means you have to wrap the radio object in an enum representing each modulation making actually using the devices a real pain...

[henrikssn]

I think this is a good idea, but I +1 that it would be better to handle this at the pure type level (so there is no runtime overhead of it).

this approach makes these traits much less usable for some of my applications as it precludes genericising over modulation types / means you have to wrap everything by modulation, and it's totally normal to run more than one configuration in an application.

as an example, i have applications testing RF links that cycle through radios, modulations, and channel configurations. at the moment i can configure a given radio then pass this into a function that takes <R: Transmit + Receive> and executes the link test, by separating the set_channel from Transmit and Receive the components that use the radio don't need to know how to configure it. we don't have any useful way to be generic over set_channel, which would definitely be useful to address!

type-state can be great for representing safety, but the overheads involved when you then have to store and mutate objects often mean it can undermine the utility of your traits. in the early radio-hal days i experimented with this to mutate the radio based on the configuration / modulation, but in practice this just means you have to wrap the radio object in an enum representing each modulation making actually using the devices a real pain...

It is not really clear to me why this leaks anything to the caller. Why wouldn't something like this work:

fn use_any_radio<Radio: Transmit<M> + Receive<M>, M: ModulationType>(radio: &mut Radio) {
  let packet = radio.receive();
  radio.transmit(packet);
}

This code snippet would work for any radio, right? We could also require defining From<ModulationType>, so that you could do:

let radio: Radio<LoRa> = MyRadio::new();
use_any_radio(radio);
let radio: Radio<Ieee802154> = radio.into();
use_any_radio(radio);
/// etc ...

Typically, the MAC layer is quite strongly tied to the physical layer (e.g. LoRa, 802.15.4, WMBus, Somfy RTS, etc etc), and I can't really find a good counter-example of this, so I don't think that converting between modulation types is going to be common in the higher layer implementations. We already have the Channel trait to change frequency within a given modulation type.

[ryankurte]

This code snippet would work for any radio, right? We could also require defining From, so that you could do:

From<ModulationType> for Radio, doesn't make a lot of sense because when you do switch modulation you need to provide a new / appropriate channel configuration anyway (ie. GFSK config != LoRA config, and you're not loading configuration for all possibilities at init). in practice you also need to store the configuration, so that either has to go into the type-state object or into an enum internally.

It is not really clear to me why this leaks anything to the caller. Why wouldn't something like this work:

as an isolated function, that is a viable workaround, however, the same problem exists in other situations, like when you go to store the radio object.

struct Mac<R: Radio<...>> {
   r: R,
}

then you have to either propagate the modulation through your container, or wrap your radio in an enum over the modulation state... really, having tried exactly this type-state approach previously it is vastly more trouble than it is worth.

Typically, the MAC layer is quite strongly tied to the physical layer (e.g. LoRa, 802.15.4, WMBus, Somfy RTS, etc etc), and I can't really find a good counter-example of this, so I don't think that converting between modulation types is going to be common in the higher layer implementations. ...

it may or may not be common in other implementations but it functionality i need, that i am currently using and have plans to continue with... it would need to be a particularly compelling argument to convince me that my use case is not a valid one 😅

[Tortoaster]

as an example, i have applications testing RF links that cycle through radios, modulations, and channel configurations. at the moment i can configure a given radio then pass this into a function that takes <R: Transmit + Receive> and executes the link test, by separating the set_channel from Transmit and Receive the components that use the radio don't need to know how to configure it. we don't have any useful way to be generic over set_channel, which would definitely be useful to address!

Would it be okay to add a generic type parameter only to Channel, or to turn the Channel associated type into a generic type parameter?

/// Channel trait for configuring radio channelization
pub trait Channel<M>
    where M: ModulationType
{
    /// Radio channel type
    type Channel: Debug;
    /// Radio error type
    type Error: Debug;

    /// Set the radio channel for future transmit and receive operations
    fn set_channel(&mut self, channel: &Self::Channel) -> Result<(), Self::Error>;
}

/// Channel trait for configuring radio channelization
pub trait Channel<C>
    where C: Debug
{
    /// Radio error type
    type Error: Debug;

    /// Set the radio channel for future transmit and receive operations
    fn set_channel(&mut self, channel: &C) -> Result<(), Self::Error>;
}

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 rust-iot/radio-hal#24), but with multiple different Channel<M> implementations, they can all coexist.

This brings one problem, namely that implementing LoRaChannel<LoRa> 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. 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.