rust-iot / rust-radio-sx127x

Rust driver for the Semtech SX127x series of Sub-GHz LoRa/ISM radio transceivers
Mozilla Public License 2.0
33 stars 16 forks source link

cargo build --no-default-features fails #5

Closed berkowski closed 4 years ago

berkowski commented 4 years ago

The crate fails to build with default features disabled (as required for use in no_std crates):

rust-radio-sx127x> cargo build --no-default-features
    Updating crates.io index
   Compiling proc-macro2 v1.0.8
   Compiling unicode-xid v0.2.0
   Compiling syn v1.0.14
   Compiling log v0.4.8
   Compiling cfg-if v0.1.10
   Compiling nb v0.1.2
   Compiling void v1.0.2
   Compiling bitflags v1.2.1
   Compiling serde v1.0.104
   Compiling libc v0.2.66
   Compiling embedded-hal v0.2.3
   Compiling embedded-spi v0.5.8
   Compiling quote v1.0.2
   Compiling async-trait v0.1.24
   Compiling serde_derive v1.0.104
   Compiling radio v0.4.4
   Compiling radio-sx127x v0.10.0 (/home/zac/Repos/rust-radio-sx127x)
error[E0599]: no method named `round` found for type `f32` in the current scope
   --> src/fsk.rs:148:64
    |
148 |         let fdev = ((channel.fdev as f32) / device::FREQ_STEP).round() as u32;
    |                                                                ^^^^^ method not found in `f32`

error[E0599]: no method named `round` found for type `f32` in the current scope
   --> src/fsk.rs:149:75
    |
149 |         let datarate = (self.config.xtal_freq as f32 / channel.br as f32).round() as u32;
    |                                                                           ^^^^^ method not found in `f32`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0599`.
error: could not compile `radio-sx127x`.

To learn more, run the command again with --verbose.

Last working tag appears to be 0.7.4, where plain integer truncation was used for fdev and datarate, e.g.

let fdev = ((channel.fdev as f32) / device::FREQ_STEP) as u32;

If rounding is absolutely required it can be provided by the libm crate with some compile-time configuration.

ryankurte commented 4 years ago

ahh whoops 🙃 thanks for the issue! iirc i changed this so behaviour was consistent with the semtech c driver so they interoperated more sensibly, could either use libm or truncate and check if the delta is > 5 to achieve the same outcome? (and either would be cool with me)

berkowski commented 4 years ago

I created a fix just for my own testing that used libm that seems to work. However, to do so I added new feature to the crate ("no-std") instead of testing against the absence of all of the existing features ("util", "serde", etc.). I'm not sure that's the right way forward but at least it confirmed that the issue is confined to these few lines.

I think truncation would be better if they're equivalent as far as the radio goes. Less complicated and more consistent across platforms.

ryankurte commented 4 years ago

to do so I added new feature to the crate ...

not that it matters at all for testing, but fyi one of the reasons to be careful of this is that there are a few situations in which cargo will union feature flags ^_^

I think truncation would be better if they're equivalent as far as the radio goes.

seems reasonable to me! I'd be happy to accept a PR if you'd like to open one, or can have a go at this later.

berkowski commented 4 years ago

Yeah, adding a "no-std" feature was just a quick hack to try the libm conditional stuff. I'll make up a quick PR with a one-off rounding function in a moment.