Add GPIO capability #406

Quantaly · 2023-06-14T17:46:13Z

This work was completed for our Advanced Software Engineering course at Colorado School of Mines.

We've added a GPIO capability with an implementation for Raspberry Pi based on rppal. The interface supports basic digital input and output as well as PWM output. A video demonstrating this functionality is attached.

demo.webm

Signed-off-by: Kai Page <[email protected]>

Signed-off-by: joeyvongphasouk <[email protected]>

Signed-off-by: brendan burmeister <[email protected]>

…for gpio unit tests. Signed-off-by: joeyvongphasouk <[email protected]>

Signed-off-by: Kai Page <[email protected]>

Stage current GPIO work

Signed-off-by: Kai Page <[email protected]>

Signed-off-by: joeyvongphasouk <[email protected]>

Signed-off-by: Kai Page <[email protected]>

PWM output for GPIO

Mossaka · 2023-06-15T16:21:07Z

Hey @Quantaly ! Thanks for your contribution! This looks pretty awesome 💙

I will admit that I am not familiar with GPIO capability, but I noted that it will only be able to run on a Raspberry Pi. This is quiet a different capability than the ones we already have, which are mostly centered on the theme of wasi-cloud-core World proposal. On the other hand, this PR does add a optional compilation feature like all other capabilities so it won't affect any downstream projects that use only a subset of SpiderLightning's capabilities.

With that being said, I am happy to merge this to the main branch, but this does not indicate that it will be part of the wasi-cloud-core World proposal. In order to become part of that, you will need to present this idea to the WASI CG meeting and then we will discuss if it makes sense to be part of the bigger World.

Since this is a pretty big PR, I will do a quick pass of review today and then let's iterate through it. I noticed that the CI is failing, could you please help to fix it?

Mossaka

Mostly look good to me but admitted I didn't run this locally yet.

I left some comments and once they are ready, I am happy to do a round 2 review!

Mossaka · 2023-06-16T17:08:00Z

wit/gpio.wit

@@ -0,0 +1,38 @@
+variant gpio-error {


Could you please add some docs on this new WIT file? What are each resources represent, much like the pwm-output-pin resource.

Mossaka · 2023-06-16T17:08:25Z

crates/gpio/src/tests.rs

+ let gpio: &mut dyn GpioImplementor = &mut gpio;
+ for config in [
+ "",
+ "some",


Mossaka · 2023-06-16T17:09:31Z

src/commands/run.rs

@@ -330,6 +335,16 @@ async fn build_store_instance(
 linked_capabilities.insert("http-client".to_string());
 }
 }
+ #[cfg(feature = "gpio")]


Could you please add at least one intergration test to this new capability so that it will be automatically tested in our CI/CD pipelines?

Mossaka · 2023-06-16T17:10:18Z

crates/gpio/Cargo.toml

+name = "slight-gpio"
+version = "0.1.0"
+edition = { workspace = true }
+authors = { workspace = true }


Please add your names to the authors of this crate

danbugs

This is looking really solid – Thank you so much for the PR, folks! Awesome work!

I've just added some comments here and there, but nothing major.

Also, to fix our CI, you might want to run:

cargo clippy --fix --all
cargo fmt --all

danbugs · 2023-06-19T22:11:11Z

crates/gpio/README.md

+ - For output: initially set to `low` or `high`
+ - For PWM output: the PWM period in microseconds, if supported by the implementation
+
+See the [example application](../../examples/gpio-demo/).


It's definitely not required to close this PR, but it might be helpful to add some comments to aid someone getting started w/ this capability – I have a Raspberry Pi at home like the one in your video, and would love to try using this (:

danbugs · 2023-06-19T22:23:26Z

crates/gpio/src/lib.rs

+ Some(Ok(_)) => Err(gpio::GpioError::PinUsageError(format!(
+ "'{name}' is not an input pin"
+ ))),
+ Some(Err(e)) => Err(e.clone()),
+ None => Err(gpio::GpioError::PinUsageError(format!(
+ "'{name}' is not a named pin"
+ ))),


This ties to my comment above about storing errors in the HashMap. I feel like a better look for this would only handle Some(<my_pin>), or None.

danbugs · 2023-06-19T22:23:51Z

crates/gpio/src/lib.rs

+ Some(Ok(Pin::Output(pin))) => Ok(pin.clone()),
+ Some(Ok(_)) => Err(gpio::GpioError::PinUsageError(format!(
+ "'{name}' is not an output pin"
+ ))),
+ Some(Err(e)) => Err(e.clone()),
+ None => Err(gpio::GpioError::PinUsageError(format!(
+ "'{name}' is not a named pin"
+ ))),


~same as above.

danbugs · 2023-06-19T22:24:01Z

crates/gpio/src/lib.rs

+ match self.pins.get(name) {
+ Some(Ok(Pin::PwmOutput(pin))) => Ok(pin.clone()),
+ Some(Ok(_)) => Err(gpio::GpioError::PinUsageError(format!(
+ "'{name}' is not a PWM output pin"
+ ))),
+ Some(Err(e)) => Err(e.clone()),
+ None => Err(gpio::GpioError::PinUsageError(format!(
+ "'{name}' is not a named pin"
+ ))),


~same as above.

danbugs · 2023-06-19T22:26:09Z

crates/gpio/src/tests.rs

+ let gpio: &mut dyn GpioImplementor = &mut gpio;
+ for config in [
+ "",
+ "some",


danbugs · 2023-06-19T22:32:09Z

wit/gpio.wit

+ enable: func() -> unit
+ /// Disable the output signal.
+ disable: func() -> unit
+}


Usually, we'd first get a proposal for an interface, and then later move on to the implementation to allow for agreement over the interface prior to any work being done – That said, I think this is looking very solid. It'd be cool to be able to add some sort of pin event handling to be able to register callbacks or smt based on specific GPIO pin events, but callback support in WIT is very restricted.

danbugs · 2023-06-19T22:34:11Z

examples/gpio-demo/slightfile.toml

+ push_down_button = "17/input/pulldown"
+ led = "5/output"
+ pwm_control_button = "18/input/pullup"
+ pwm_led = "6/pwm_output"


I see that you're defining pin config. (i.e., pull-up/pull-down) directly in the string vars here. Have you considered baking this into some sort of structure within the interface?

danbugs · 2023-06-19T22:37:00Z

crates/gpio/src/lib.rs

+ let pins = state
+ .configs_map
+ .map(|configs| {
+ configs
+ .iter()
+ .map(|(name, config)| (name.clone(), implementor.parse_pin_config(config)))
+ .collect()
+ })
+ .unwrap_or_else(HashMap::new);


Currently, all pin usage is registered via the slightfile – Is there any usage to registering pins are runtime?

danbugs · 2023-06-19T22:39:08Z

crates/gpio/src/lib.rs

+/// It must be [Send], [Sync], and [Clone].
+#[derive(Clone)]
+pub struct Gpio {
+ pins: HashMap<String, Result<Pin, gpio::GpioError>>,


Are we sure we want to store possible errors in the HashMap? As a user, I'd rather fail at compile-time instead of trying to get my pin and failing at runtime.

danbugs · 2023-09-11T16:13:24Z

@Quantaly , @joeyvongphasouk , and @BrendanBurm ~ Is this something you are still interested in merging?

Quantaly and others added 30 commits May 30, 2023 12:05

Basic GPIO WIT

a540a1d

Signed-off-by: Kai Page <[email protected]>

Starting work on GPIO

a54cc53

Signed-off-by: Kai Page <[email protected]>

GPIO: Raspberry Pi pin construction

bc18e21

Signed-off-by: Kai Page <[email protected]>

Remove the RaspberryPiImplementorInner struct

f4ef59b

Signed-off-by: Kai Page <[email protected]>

Implement Raspberry Pi GPIO methods

f64c422

Signed-off-by: Kai Page <[email protected]>

Actually call the pin destructor

5f424d4

Signed-off-by: Kai Page <[email protected]>

Applications reference GPIO pins by name

ab486e3

Signed-off-by: Kai Page <[email protected]>

GPIO configuration

f5b0f31

Signed-off-by: Kai Page <[email protected]>

Add pullup/pulldown and initial output state

659a9e5

Signed-off-by: Kai Page <[email protected]>

Unit tests for pin configuration parsing

87c201d

Signed-off-by: Kai Page <[email protected]>

Made initial demo of input(pushdown) and output

4445bf4

Signed-off-by: joeyvongphasouk <[email protected]>

Added cargo dependencies

33adb2e

Signed-off-by: joeyvongphasouk <[email protected]>

added flicker to gpiodemo

ddd56a7

Signed-off-by: brendan burmeister <[email protected]>

Added additional documentation onto gpio demo. Added more test cases …

400e0c5

…for gpio unit tests. Signed-off-by: joeyvongphasouk <[email protected]>

Added documentation to tests.rs and lib.rs in gpio crate

f328c0e

Write documentation for GPIO capability stuff

b77b6e7

Signed-off-by: Kai Page <[email protected]>

Merge my GPIO documentation with Brandon's

5067c6c

Signed-off-by: Kai Page <[email protected]>

Merge pull request #1 from mines-370-wasm-squad/gpio-dev

424a3fc

Stage current GPIO work

Merge remote-tracking branch 'upstream/main' into gpio

f0f022e

Add PWM output pin to GPIO

031a844

Signed-off-by: Kai Page <[email protected]>

Update GPIO demo with PWM

3ad4258

Signed-off-by: Kai Page <[email protected]>

Fix bug in GPIO demo

76fbde2

Signed-off-by: Kai Page <[email protected]>

Fix other bug in GPIO demo

1935c41

Signed-off-by: Kai Page <[email protected]>

Specify type with annotation instead of cast

4f786fe

Signed-off-by: Kai Page <[email protected]>

Revised documentation of gpio demo

cd18c2b

Signed-off-by: joeyvongphasouk <[email protected]>

Update GPIO documentation

d7287c6

Signed-off-by: Kai Page <[email protected]>

Merge documentation updates

c588203

Signed-off-by: Kai Page <[email protected]>

Use crate doc comment syntax

cccea05

Signed-off-by: Kai Page <[email protected]>

Emit warnings instead of errors for PWM

e53a187

Signed-off-by: Kai Page <[email protected]>

Merge pull request #2 from mines-370-wasm-squad/gpio-pwm

210aa31

PWM output for GPIO

Mossaka reviewed Jun 16, 2023

View reviewed changes

danbugs reviewed Jun 19, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GPIO capability #406

Add GPIO capability #406

Quantaly commented Jun 14, 2023

Mossaka commented Jun 15, 2023

Mossaka left a comment

Mossaka Jun 16, 2023

Mossaka Jun 16, 2023

danbugs Jun 19, 2023

Mossaka Jun 16, 2023

Mossaka Jun 16, 2023

danbugs left a comment

danbugs Jun 19, 2023

danbugs Jun 19, 2023

danbugs Jun 19, 2023

danbugs Jun 19, 2023

danbugs Jun 19, 2023

danbugs Jun 19, 2023

danbugs Jun 19, 2023

danbugs Jun 19, 2023

danbugs Jun 19, 2023

danbugs commented Sep 11, 2023

Add GPIO capability #406

Are you sure you want to change the base?

Add GPIO capability #406

Conversation

Quantaly commented Jun 14, 2023

Mossaka commented Jun 15, 2023

Mossaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danbugs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danbugs commented Sep 11, 2023