LoRa examples and testing

[rust-iot]

LoRa examples and testing

[pdgilbert]

I now have examples mostly working in branch examples-testing using embedded-hal v1.0.0-alpha.4. In several places the example code uses something like

use radio_sx127x::Error as sx127xError;     // Error name conflict with hals
...
fn setup() ->  impl DelayMs<u32> + Transmit<Error=sx127xError<Error, Infallible, Infallible>> {

I hope it should be possible to replace some of this with something that makes the example more readable, but I have not had any luck. I think Infallible, Infallible should be something like two of CommsError, PinError, DelayError, but have not figure out which to use and where to import from.

Any suggestions?

[ryankurte]

I think Infallible, Infallible should be something like two of CommsError, PinError, DelayError, but have not figure out which to use and where to import from.

this reflects the error type on the spi and gpio traits, likely with the device you're using they are actually infallible (this is not always the case with linux or other higher-level HALs) otherwise the type bounds would be wrong / wouldn't be compiling, so, you should be good there!

[pdgilbert]

I was running rustfmt to simplify reconciling some differences with my https://pdgilbert.github.io/eg_stm_hal/#table-of-additional-examples-status versions of the examples, and decided to run clippy (cargo clippy --no-default-features --features=stm32f4xx,stm32f411 -- -D warnings ) just for the pain of it. There are lots of suggestions, well beyond me, and will probably not make any difference, but one thing that may be worth attention is

error: float has excessive precision
   --> src/device/mod.rs:229:28
    |
229 | pub const FREQ_STEP: f32 = 61.03515625;
    |                            ^^^^^^^^^^^ help: consider changing the type or truncating it to: `61.035_156`
    |

Is the frequency step not sensitive to that extra 25? And I'm not sure of the units for frequency step but maybe code size could be improved by scaling (MHz to Hz)? As you can guess, this is another thing not in my area of expertise.

[pdgilbert]

I don't yet understand dependabot. Is it looking at Cargo.lock to see what can be updated within the specification in Cargo.toml, or does it test updates beyond the Cargo.toml specification?

In any case, there may not be any point in having it run on my fork. I would get any of those updates by rebasing to upstream.

[pdgilbert]

OTOH, my examples provide some integration testing, so maybe it is better to keep dependabot running on my fork?

[ryankurte]

I don't yet understand dependabot. Is it looking at Cargo.lock to see what can be updated within the specification in Cargo.toml, or does it test updates beyond the Cargo.toml specification?

i think it attempts to update past the toml constraints as i've seen requests for tokio 0.2 to 1.0 on other repos.

OTOH, my examples provide some integration testing, so maybe it is better to keep dependabot running on my fork?

depends on your goal with the fork? i'd be happy to have the examples merged back if you're interested in that, in which case it's probably moot.

[ryankurte]

Is the frequency step not sensitive to that extra 25? And I'm not sure of the units for frequency step but maybe code size could be improved by scaling (MHz to Hz)? As you can guess, this is another thing not in my area of expertise.

ahh it probably is, though I am not sure how sensitive. i intended to refactor this to be fixed-point math with u32/u64s based on the definition in the datasheet, something like this but haven't yet had the opportunity to look at it.

[pdgilbert]

i'd be happy to have the examples merged back if you're interested in that, in which case it's probably moot.

Yes, I'm hoping you will merge them. But there are downsides. One is that the workflow takes longer to build. Another is that there are a lot more dependencies. I've made them optional because if one of them fails to build then the package does not build. They really should be dev dependencies because they are only used in examples, but I have not figured out how to make a dev dependency optional.

I'm just doing some final cleanup and will do a PR sometime soon.

[pdgilbert]

I have been trying to get a variation of my lora_spi_send example working elsewhere and have to use my version of rust-radio-sx127x rather than release or git rust-iot/rust-radio-sx127x or ryankurte/rust-radio-sx127x, otherwise I get the trait bound problem

  Compiling eg_stm_hal v0.1.0 (/home/paul/githubClones/eg_stm_hal/boards/blackpill-stm32f411)
error[E0277]: the trait bound `radio_sx127x::Sx127x<driver_pal::wrapper::Wrapper<Compat<Spi<stm32f4xx_hal::stm32::SPI1, (PA5<Alternate<stm32f4xx_hal::gpio::AF5>>, PA6<Alternate<stm32f4xx_hal::gpio::AF5>>, PA7<Alternate<stm32f4xx_hal::gpio::AF5>>)>>, stm32f4xx_hal::spi::Error, Compat<PA1<Output<PushPull>>>, Compat<PB8<Input<Floating>>>, Compat<PB9<Input<Floating>>>, Compat<PA0<Output<PushPull>>>, Infallible, Compat<stm32f4xx_hal::delay::Delay>, Infallible>, stm32f4xx_hal::spi::Error, Infallible, Infallible>: Transmit` is not satisfied
   --> examples/zzlora_spi_send.rs:312:15
    |
312 | fn setup() -> impl DelayMs<u32> + Transmit<Error = sx127xError<Error, Infallible, Infallible>> {
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Transmit` is not implemented for `radio_sx127x::Sx127x<driver_pal::wrapper::Wrapper<Compat<Spi<stm32f4xx_hal::stm32::SPI1, (PA5<Alternate<stm32f4xx_hal::gpio::AF5>>, PA6<Alternate<stm32f4xx_hal::gpio::AF5>>, PA7<Alternate<stm32f4xx_hal::gpio::AF5>>)>>, stm32f4xx_hal::spi::Error, Compat<PA1<Output<PushPull>>>, Compat<PB8<Input<Floating>>>, Compat<PB9<Input<Floating>>>, Compat<PA0<Output<PushPull>>>, Infallible, Compat<stm32f4xx_hal::delay::Delay>, Infallible>, stm32f4xx_hal::spi::Error, Infallible, Infallible>`

The difference when I change to mine is

    Removing async-trait v0.1.48
    Removing radio v0.7.0
      Adding radio-sx127x v0.10.1 (https://github.com/pdgilbert/rust-radio-sx127x#fabd2df8)
    Removing radio-sx127x v0.10.1 (https://github.com/rust-iot/rust-radio-sx127x#8a976033)

coming from

$ cargo tree -i -p radio:0.7.0
radio v0.7.0
└── radio-sx127x v0.10.1 (https://github.com/rust-iot/rust-radio-sx127x#8a976033)
    └── eg_stm_hal v0.1.0 (/home/paul/githubClones/eg_stm_hal/boards/blackpill-stm32f411)

It might be worth fixing this sooner if the PR for examples is going to take awhile.

[ryankurte]

hmm, different versions of radio? what does cargo tree -i radio say?

Yes, I'm hoping you will merge them...

i'd certainly be up to have a couple of examples here, though i am not sure from a library perspective it makes sense to maintain more than that here. that said i think your idea of having a bunch of working examples across different platforms is excellent, perhaps this could be a separate project that uses a collection of drivers (eg. a matrix of platforms and drivers)?

[pdgilbert]

My Cargo.toml has

# for  lora_spi_*
radio-sx127x  = { version = "0.10.1",        default-features = false }
#radio-sx127x  = {  git = "https://github.com/rust-iot/rust-radio-sx127x", default-features = false }
#radio-sx127x  = {  git = "https://github.com/pdgilbert/rust-radio-sx127x", default-features = false }
radio         = { version = "0.8.1" }
driver-pal    = { version = "0.8.0-alpha.0", default-features = false }

My example works with my git version.

$ cargo tree -i radio
radio v0.8.1 (https://github.com/ryankurte/rust-radio.git?branch=master#b2383d55)
├── eg_stm_hal v0.1.0 (/home/paul/githubClones/eg_stm_hal/boards/none-stm32h742)
└── radio-sx127x v0.10.1 (https://github.com/pdgilbert/rust-radio-sx127x#fabd2df8)
    └── eg_stm_hal v0.1.0 (/home/paul/githubClones/eg_stm_hal/boards/none-stm32h742)

If I switch to your git version and cargo update

...
    Updating git repository `https://github.com/stm32-rs/stm32l4xx-hal`
      Adding async-trait v0.1.48
    Removing embedded-hal-compat v0.1.2 (https://github.com/ryankurte/embedded-hal-compat.git?branch=main#e58fd8ff)
      Adding radio v0.7.0
    Removing radio-sx127x v0.10.1 (https://github.com/pdgilbert/rust-radio-sx127x#fabd2df8)
      Adding radio-sx127x v0.10.1 (https://github.com/rust-iot/rust-radio-sx127x#8a976033)

then I have impl trait problems and

$ cargo tree -i radio
error: There are multiple `radio` packages in your project, and the specification `radio` is ambiguous.
Please re-run this command with `-p <spec>` where `<spec>` is one of the following:
  radio:0.7.0
  radio:0.8.1
$ cargo tree -i radio:0.7.0
radio v0.7.0
└── radio-sx127x v0.10.1 (https://github.com/rust-iot/rust-radio-sx127x#8a976033)
    └── eg_stm_hal v0.1.0 (/home/paul/githubClones/eg_stm_hal/boards/none-stm32h742)
paul@zbook ~/rust/eg_stm_hal/boards/none-stm32h742 $ cargo tree -i radio:0.8.1
radio v0.8.1 (https://github.com/ryankurte/rust-radio.git?branch=master#b2383d55)
└── eg_stm_hal v0.1.0 (/home/paul/githubClones/eg_stm_hal/boards/none-stm32h742)

If I then switch to release version and cargo update

...
    Updating git repository `https://github.com/stm32-rs/stm32l4xx-hal`
      Adding embedded-spi v0.5.8
    Updating radio v0.7.0 -> v0.4.4
      Adding radio-sx127x v0.10.1
    Removing radio-sx127x v0.10.1 (https://github.com/rust-iot/rust-radio-sx127x#8a976033)

I still have impl trait problems, and different radio versions

error: There are multiple `radio` packages in your project, and the specification `radio` is ambiguous.
Please re-run this command with `-p <spec>` where `<spec>` is one of the following:
  radio:0.4.4
  radio:0.8.1
$ cargo tree -i radio:0.4.4
radio v0.4.4
└── radio-sx127x v0.10.1
    └── eg_stm_hal v0.1.0 (/home/paul/githubClones/eg_stm_hal/boards/none-stm32h742)
paul@zbook ~/rust/eg_stm_hal/boards/none-stm32h742 $ cargo tree -i radio:0.8.1
radio v0.8.1 (https://github.com/ryankurte/rust-radio.git?branch=master#b2383d55)
└── eg_stm_hal v0.1.0 (/home/paul/githubClones/eg_stm_hal/boards/none-stm32h742)

The whole tree is messy because this is for several examples

Click to expand

$ cargo tree
eg_stm_hal v0.1.0 (/home/paul/githubClones/eg_stm_hal/boards/none-stm32h742)
├── asm-delay v0.7.2
│   ├── bitrate v0.1.1
│   ├── cortex-m v0.7.2
│   │   ├── bare-metal v0.2.5
│   │   │   [build-dependencies]
│   │   │   └── rustc_version v0.2.3
│   │   │       └── semver v0.9.0
│   │   │           └── semver-parser v0.7.0
│   │   ├── bitfield v0.13.2
│   │   ├── embedded-hal v0.2.4
│   │   │   ├── nb v0.1.3
│   │   │   │   └── nb v1.0.0
│   │   │   └── void v1.0.2
│   │   └── volatile-register v0.2.0
│   │       └── vcell v0.1.3
│   └── embedded-hal v0.2.4 (*)
├── cortex-m v0.7.2 (*)
├── cortex-m-rt v0.6.13
│   ├── cortex-m-rt-macros v0.1.8 (proc-macro)
│   │   ├── proc-macro2 v1.0.24
│   │   │   └── unicode-xid v0.2.1
│   │   ├── quote v1.0.9
│   │   │   └── proc-macro2 v1.0.24 (*)
│   │   └── syn v1.0.64
│   │       ├── proc-macro2 v1.0.24 (*)
│   │       ├── quote v1.0.9 (*)
│   │       └── unicode-xid v0.2.1
│   └── r0 v0.2.2
├── dht-sensor v0.2.1
│   └── embedded-hal v0.2.4 (*)
├── driver-pal v0.8.0-alpha.2
│   ├── embedded-hal v1.0.0-alpha.4
│   │   └── nb v1.0.0
│   └── log v0.4.14
│       └── cfg-if v1.0.0
├── embedded-graphics v0.6.2
│   └── byteorder v1.4.3
├── embedded-hal v0.2.4 (*)
├── embedded-hal v1.0.0-alpha.4 (*)
├── embedded-hal-compat v0.1.3
│   ├── embedded-hal v0.2.4 (*)
│   └── embedded-hal v1.0.0-alpha.4 (*)
├── embedded-spi v0.6.2
│   ├── embedded-hal v0.2.4 (*)
│   └── log v0.4.14 (*)
├── mcp4x v0.1.0 (https://github.com/eldruin/mcp4x-rs#da0b047e)
│   └── embedded-hal v0.2.4 (*)
├── nb v1.0.0
├── panic-halt v0.2.0
├── panic-reset v0.1.0
│   └── cortex-m v0.6.7
│       ├── aligned v0.3.4
│       │   └── as-slice v0.1.5
│       │       ├── generic-array v0.12.4
│       │       │   └── typenum v1.13.0
│       │       ├── generic-array v0.13.3
│       │       │   └── typenum v1.13.0
│       │       ├── generic-array v0.14.4
│       │       │   └── typenum v1.13.0
│       │       │   [build-dependencies]
│       │       │   └── version_check v0.9.3
│       │       └── stable_deref_trait v1.2.0
│       ├── bare-metal v0.2.5 (*)
│       ├── bitfield v0.13.2
│       ├── cortex-m v0.7.2 (*)
│       └── volatile-register v0.2.0 (*)
├── radio v0.8.1 (https://github.com/ryankurte/rust-radio.git?branch=master#b2383d55)
│   ├── chrono v0.4.19
│   │   ├── num-integer v0.1.44
│   │   │   └── num-traits v0.2.14
│   │   │       [build-dependencies]
│   │   │       └── autocfg v1.0.1
│   │   │   [build-dependencies]
│   │   │   └── autocfg v1.0.1
│   │   └── num-traits v0.2.14 (*)
│   ├── embedded-hal v1.0.0-alpha.4 (*)
│   ├── log v0.4.14 (*)
│   └── nb v1.0.0
├── radio-sx127x v0.10.1
│   ├── bitflags v1.2.1
│   ├── embedded-hal v0.2.4 (*)
│   ├── embedded-spi v0.5.8
│   │   ├── embedded-hal v0.2.4 (*)
│   │   └── log v0.4.14 (*)
│   ├── libc v0.2.90
│   ├── log v0.4.14 (*)
│   └── radio v0.4.4
│       ├── async-trait v0.1.48 (proc-macro)
│       │   ├── proc-macro2 v1.0.24 (*)
│       │   ├── quote v1.0.9 (*)
│       │   └── syn v1.0.64 (*)
│       ├── embedded-hal v0.2.4 (*)
│       ├── log v0.4.14 (*)
│       └── nb v0.1.3 (*)
├── ssd1306 v0.5.1 (https://github.com/jamwaffles/ssd1306#23bd9b05)
│   ├── display-interface v0.4.0
│   ├── display-interface-i2c v0.4.0
│   │   ├── display-interface v0.4.0
│   │   └── embedded-hal v0.2.4 (*)
│   ├── display-interface-spi v0.4.0
│   │   ├── byte-slice-cast v0.3.5
│   │   ├── display-interface v0.4.0
│   │   └── embedded-hal v0.2.4 (*)
│   ├── embedded-graphics v0.6.2 (*)
│   ├── embedded-hal v0.2.4 (*)
│   └── generic-array v0.14.4 (*)
└── void v1.0.2
[dev-dependencies]
├── cortex-m-semihosting v0.3.7
│   └── cortex-m v0.7.2 (*)
├── heapless v0.6.1
│   ├── as-slice v0.1.5 (*)
│   ├── generic-array v0.14.4 (*)
│   ├── hash32 v0.1.1
│   │   └── byteorder v1.4.3
│   └── stable_deref_trait v1.2.0
├── panic-halt v0.2.0
└── panic-semihosting v0.5.6
    ├── cortex-m v0.7.2 (*)
    └── cortex-m-semihosting v0.3.7 (*)
</details>

[pdgilbert]

i think your idea of having a bunch of working examples across different platforms is excellent, perhaps this could be a separate project that uses a collection of drivers (eg. a matrix of platforms and drivers)?

I've been working on this for awhile. The Travis CI currently using embedded-hal 0.2.4 is summarized at https://pdgilbert.github.io/eg_stm_hal/. rust-radio-sx127x does not work on that but I now have a Github workflow running on a branch e-h-1.0.0alpha-compat, see https://github.com/pdgilbert/eg_stm_hal/actions. (Cargo.toml used above is from this.)

[pdgilbert]

i'd certainly be up to have a couple of examples here, though i am not sure from a library perspective it makes sense to maintain more than that here.

Ok, I would like stm32f411 because it is the MCU I can most easily run at the moment. My next two choices would be stm32f103 (bluepill) because many people have one, and stm32f303 because it also seems to be well used. OTOH, there seems to be a lot of activity on stm32h7xx too. I cannot actually run test on that, but there are probably better qualified people around that can. So, what would be your preference?

[ryankurte]

stm32f411 sounds great to me! nice ICs and iirc that's the blackpill micro so it's pretty available.

[pdgilbert]

While trying to test when I have everything on e-h-1.0.0-alpha.4 and do not need embedded-hal-compat I discovered that it is included anyway because of the dependency in my fork's rust-radio-sx127x, and if I make it optional then it seems to be missing when I need it. A way around this seems to be to have a feature compat = ["embedded-hal-compat"]. Is there anything else you would suggest?

[ryankurte]

hmm, you shouldn't need the compat stuff at all if you're using e-h@1.0.0-alpha.N? and it -should- be injectable at the top level rather than in the sx127x project

[pdgilbert]

Yes, that's what I expected, and what I was testing. But in order to make examples work in my fork of rust-radio-sx127x I had

embedded-hal-compat = { git = "https://github.com/ryankurte/embedded-hal-compat.git", branch = "main"}

which I have now made optional so it is not pulled in when not needed. But in order for it to get pulled in when I need it for examples in the fork I added

compat   = ["embedded-hal-compat"]

Maybe there is a better way?

[pdgilbert]

This issue is out-of-date and effectively resolved.