Expose low-level interface.

[qwandor]

Sometimes a user of the library may want to use the high-level interface for most things, but set some other registers which are not currently supported in the high-level interface. This lets them combine the high-level interface with the low-level interface when necessary.

[SebKuzminsky]

I made this same change locally, having access to the lowlevel stuff is convenient. I suggest also making Cc1101::await_machine_state() public.

[dsvensson]

Please motivate "sometimes" with an example that doesn't also reveal a too limited API.

[dsvensson]

Await state machine is a hack, should probably expose a countdown that seems available via Linux embedded hal these days.

[SebKuzminsky]

Like @qwandor I wanted to use the low-level interface to access register functionality not yet exposed by the high-level interface. And in that situation, await_state_machine() is occasionally a requirement, e.g.:

// start calibration, then wait for calibration to complete                                                                      
cc1101_handle.0.write_strobe(cc1101::lowlevel::registers::Command::SCAL).unwrap();                    
cc1101_handle.await_machine_state(cc1101::lowlevel::types::MachineState::IDLE).unwrap();              

I think you're advocating keeping lowlevel private, and instead expanding the highlevel API to provide all functionality that users want. That's a good goal, but there is so much functionality in the CC1101 that it's a large task...

My application sends a small packet, and it requires writing 36 registers in order to configure the chip right. It was a lot more expedient to do the register writes via the lowlevel interface than writing wrappers for each field in the high-level interface.

[SebKuzminsky]

Something like this public lowlevel idea was mentioned in #21.

[dsvensson]

I think this action should be a function with a name, and take a generic timer param like many other things. The current await_state_machine is an atrocity as the linux-embedded-hal was underdeveloped at the time.

[qwandor]

Right, I agree that exposing all functionality in the high-level API is a good goal, but it's unlikely that that will actually happen anytime soon given how many different options there are so having access to the low-level API is a useful way around in the meantime. Otherwise anyone who wants to change some option which is not yet exposed needs to send a PR and wait for another release before they can start doing it in their application. I've also added wrappers for the functionality I've found necessary so far in #33, but I expect there will be more tuning necessary.

[dsvensson]

It's the same issue with addons in games. The reason world of warcraft hasn't evolved is because of the addon system. The idea of this project is a revolt against all the bazillion cc1xxx project in go, c, c++ etc put there that all have a function where they just have a init function that writes a big hex blob to init things, that has been chain-copied countless times to reach some kind of audiophelia level. What might feel annoying is known as progress. But you are free to fork and choose your own direction.

[SebKuzminsky]

@dsvensson

you are free to fork and choose your own direction

I'd prefer to collaborate on a single best-of-breed library, i'm not willing to fork based on this disagreement.

[Sympatron]

What about this:

pub unsafe fn lowlevel(&mut self) -> &mut lowlevel::Cc1101<SPI, CS> {
    &mut self.0
}

From a memory safety perspective this isn't technically unsafe. But this could convey your worries about it being used to freely (if I understand you correctly). You could even document that this API is unstable and could be removed in future versions, when this crate supports a bigger subset of the CC1101's functionality. Or it could be an optional feature you would have to opt-in.

I fully agree that it would be better to have safe and tested abstractions, but one might just want to quickly test a few different settings before committing to an appropriate abstraction.

It's really not worth it splitting this crate into forks over this IMO.

[dsvensson]

closed due to stale with conflicts