Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More hardware support, new features, new examples #11

Closed
wants to merge 0 commits into from

Conversation

joakim000
Copy link
Contributor

Hub support

Accessing devices through the hub has changed; instead of a fixed port map, the map connected_io is populated with attached devices and their available options, and we can select a device from there. The io_from_.. methods wrap some useful calls on connected_io(), for example;

  • io_from_kind(IoTypeId::HubLed)
    accesses the LED on any hub type though hardware addresses differ, and
  • io_multiple_from_kind(IoTypeId::Motor)
    accesses all motors indifferent to where they are connected.

There is less need for specific hub support; a generic hub implementation
is tested with:

  • Technic Medium hub, aka. Control+
  • Move hub, aka. Boost
  • Remote control handset

Other hubs that likewise use LWP 3.0 should just work.
Also enabled commands for Hub Actions, Hub Properties and Hub Alerts that was missing serialisation.

Device support

Basic device trait gives access to low level mode set and output commands.
Generic sensor trait gives one-liners for setting device modes and returning a channel receiver.
Specific support traits for encoder motors (position/angle commands, speed and position feedback), the 28739 remote
handset and 26912 multi-sensor.

Examples

The previous examples have been updated for api changes. New examples:

  • device-info
    Dev util; display configuration details for devices and interactively
    set device modes. With a colourful easter egg.
  • remote-control
    Setup and basic demo of the 28739 remote handset.
  • motor-closed-loop
    Setup for closed loop motor control. Demo uses position feedback
    to enforce software defined travel limits.
    Additionally shows connection to multiple hubs (main hub and rc).
  • vision-sensor
    Setup and demo of the 26912 multi-functional sensor; interactive mode
    selection.

Redesigned device abstraction

Devices have been factored into: a struct Definition that holds available configuration data and related functions, and: specific device support as default functions in device traits, that a device type may implement to access the functionality.

This is currently held together in the simplest way; a single IoDevice type that implements all device traits, with some runtime checks for compatibility. The actual device kind is currently only encoded as a IoTypeId variant in Definition.kind - it would be preferable to encode in types that implement only relevant device traits to get compile checking of usage. Separated abstractions for device data (Definition) and Lego-interfacing methods (device traits) should hopefully facilitate improvement.

@sciguy16
Copy link
Member

Wow, thanks for this! It'll take me some time to go through the changes and test them out, but in the meantime if you'd be able to get it passing CI that'd be greatly appreciated

@joakim000
Copy link
Contributor Author

Sure, it was my pleasure. I would like to work on nostd-support as well, but I'm still very new to Rust so it may be beyond me for the moment. I'll post an issue on that later. Sorry about the CI-fails, should pass now.

Copy link
Member

@sciguy16 sciguy16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work, thank you! I really like the IoDevice APIs and support for sensors. Just suggested a few changes - if you've got some time to have a look at those and then I'll get it merged & publish a new release to crates.io

deploy.sh Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this for? If it's WSL-specific then please name it something like 'build-wsl.sh' and put a comment at the top to explain what it's for

.cargo/config Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed for this PR or is it for some nostd embedded work?

@@ -1,15 +1,21 @@
[package]
name = "gamepad"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to keep the gamepad example - as long as the code compiles it will probably still work. I'm happy to test that the controller support still works

dbus = {version = "0.9.7", features = ["vendored"], optional = true}

[features]
wslcross = ["dep:dbus"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you add a note to the README explaining how to build/run on WSL?

examples/device-info/Cargo.toml Outdated Show resolved Hide resolved
lego-powered-up/src/setup.rs Outdated Show resolved Hide resolved
lego-powered-up/src/tests.rs Outdated Show resolved Hide resolved
@@ -25,7 +25,7 @@ async fn main() -> Result<()> {
.init();

match args.command {
Command::Devices(dev_args) => devices::run(&dev_args).await?,
Command::Devices(dev_args) => adapters::run(&dev_args).await?,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to rename Command::Devices to Command::Adapters?

.gitignore Outdated Show resolved Hide resolved
lego-powered-up/src/iodevice.rs Outdated Show resolved Hide resolved
@sciguy16
Copy link
Member

Wil need to have a think about how best to approach no_std support in a separate ticket - in the year or so since I last thought about it properly the state of BLE support in embedded Rust has probably changed a lot!

.gitignore Outdated
@@ -1 +1,4 @@
/target
Cargo.lock
Copy link
Contributor

@romw314 romw314 Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Cargo.lock

Probably don't need to gitignore Cargo.lock:
https://stackoverflow.com/questions/43667176/what-files-in-a-cargo-project-should-be-in-my-gitignore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants