Open ryankurte opened 2 years ago
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.
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.
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.)
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.
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. :grin:
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 forLoRaWAN
on channel 3, or802.15.4g
on channel 2, orBle5.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. :slightly_smiling_face:
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.
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.
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.
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.
mostly for RX the info is
RSSI
and maybe anLQI
, it doesn't substantially vary with modulation. i've tended to just useBasicInfo
and set thelqi
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 aLoRaInfo
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 inradio
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 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.
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 :yum:
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 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.
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
e-h@1.0.0-alpha
but, i don't have it in me to update everything rn)cc. @henrikssn