From 33f7e363a628f987a90d43c4279dd3d2e4a32f9d Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Sun, 17 Nov 2019 19:26:37 +0300 Subject: [PATCH 1/9] digital::v3 RFC --- rfcs/0393-digital-v3.md | 97 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 rfcs/0393-digital-v3.md diff --git a/rfcs/0393-digital-v3.md b/rfcs/0393-digital-v3.md new file mode 100644 index 00000000..35dae9de --- /dev/null +++ b/rfcs/0393-digital-v3.md @@ -0,0 +1,97 @@ +- Feature Name: digital_v3 +- Start Date: 2019-11-17 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +This RFC proposes `digital::v3` fallible interface and un-deprecation of `digital::v1` interface. + +# Motivation +[motivation]: #motivation + +Current `digital::v2` interface has some problems: + +* `digital::v2` interface has the same method names as `digital::v1` interface, +due to this fact they cannot be used together and `digital::v2` traits in prelude may break things. +* Current implementation deprecates `digital::v1` in favor of `digital::v2` interface, +this has unforeseen consequences for HAL libraries. +As a result, user code suffers from useless `.unwrap()` constructions. + +This RFC aims to solve these problems. + +Currently, `embedded-hal` digital traits are used from three perspectives: +* driver developers +* hal developers +* end-users + +`digital::v2` traits were introduced to provide driver developers with a way to work with +potentially fallible GPIO pins. Such pins can be created by drivers for external GPIO expanders connected +via fallible peripherals (e.g. i2c). At the same time, most MCUs do not have fallible GPIOs and hals are not +required to implement fallible `digital::v2` interface for them. + +Due to the deprecation of `digital::v1` traits, hal maintainers were annoyed by deprecation warnings and +switched to `digital::v2` traits to get rid of these warnings. This change introduced another problem: +end-users started getting warnings about unused result for the new (now `digital::v2`) GPIO methods. +These warnings introduced the `.unwrap()` pattern for using GPIO methods, even for GPIOs provided by MCU hal +libraries. Since most embedded projects do not use external fallible GPIOs, this led to +a decrease in code readability. + +From the end-user perspective, co-existence of `digital::v2` and `digital::v1` traits led to another problem: +since all `digital::v1` implicitly implement `digital::v2` traits, they implement two different +traits with the same method names. When both traits are in scope, calls to any of the `digital::v1` methods +produce an error due to the ambiguity of used trait. Due to this fact, one also can't have both traits in +`embedded-hal` prelude for convenience. + +# Detailed design +[design]: #detailed-design + +This RFC proposes the following changes: + +* Add `digital::v3` interface with the same traits as `digital::v2`, but with methods named with `try_` prefix. +For example, `ToggleableOutputPin` will have `try_toggle` method instead of `toggle`. +* Provide transparent conversion from `digital::v2` traits to `digital::v3` traits. +* Change `digital::v1` compatibility layer to convert from `digital::v3` traits instead of `digital::v2` ones. +* Add `V2OutputPin` proxy to the `digital::v2` compatibility layer to provide `digital::v3` -> `digital::v2` downgrade. +* Add `digital::v3` traits to prelude. +* Un-deprecate `digital::v1` traits, add a note for driver developers. +* Deprecate `digital::v2` traits in favor of `digital::v3` traits. +* Move HALs back to `digital::v1` interface. + +### Consequences + +* End users will be able to use both fallible and infallible traits and import them via `embedded-hal` prelude. +* HALs will export GPIOs as infallible (as they really are). +* Driver developers can move to fallible `digital::v3` traits with minimal effort. +* End users that use drivers with `digital::v2` interface will continue using them without any changes in +most of the cases. Newer `digital::v3` objects could be passed to `digital::v2` consumers via `V2OutputPin` shim. + +# How We Teach This +[how-we-teach-this]: #how-we-teach-this + + + +What names and terminology work best for these concepts and why? +How is this idea best presented—as a continuation of existing Rust patterns, or as a wholly new one? + +Would the acceptance of this proposal change how Rust is taught to new users at any level? +How should this feature be introduced and taught to existing Rust users? + +What additions or changes to the Rust Reference, _The Rust Programming Language_, and/or _Rust by Example_ does it entail? + +# Drawbacks +[drawbacks]: #drawbacks + +The proposed approach does not provide hal and driver developers with a warning about the interface they should use. + +# Alternatives +[alternatives]: #alternatives + +Changing `digital::v2` interface to provide different method names in all the pin traits. +This solves the name clashing problem, but does not solve `digital::v1` deprecation problem. + +# Unresolved questions +[unresolved]: #unresolved-questions + +Naming the methods: while `try_set_high` and `try_toggle` seem ok, `try_is_set_high` name is a bit weird. From 19056c040102476ccd048f65a254233c8d79680a Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Sun, 17 Nov 2019 19:35:13 +0300 Subject: [PATCH 2/9] Update 'How We Teach This' section --- rfcs/0393-digital-v3.md | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/rfcs/0393-digital-v3.md b/rfcs/0393-digital-v3.md index 35dae9de..b3c986ad 100644 --- a/rfcs/0393-digital-v3.md +++ b/rfcs/0393-digital-v3.md @@ -70,15 +70,7 @@ most of the cases. Newer `digital::v3` objects could be passed to `digital::v2` # How We Teach This [how-we-teach-this]: #how-we-teach-this - - -What names and terminology work best for these concepts and why? -How is this idea best presented—as a continuation of existing Rust patterns, or as a wholly new one? - -Would the acceptance of this proposal change how Rust is taught to new users at any level? -How should this feature be introduced and taught to existing Rust users? - -What additions or changes to the Rust Reference, _The Rust Programming Language_, and/or _Rust by Example_ does it entail? +Intended use of each pin interface in different contexts should be clearly indicated in the `embedded-hal` docs. # Drawbacks [drawbacks]: #drawbacks From 28a22faaf3b001b0b0af94b98b3e9600416a82bd Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Mon, 18 Nov 2019 01:00:27 +0300 Subject: [PATCH 3/9] Mention multiple error problem --- rfcs/0393-digital-v3.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rfcs/0393-digital-v3.md b/rfcs/0393-digital-v3.md index b3c986ad..f053c641 100644 --- a/rfcs/0393-digital-v3.md +++ b/rfcs/0393-digital-v3.md @@ -87,3 +87,6 @@ This solves the name clashing problem, but does not solve `digital::v1` deprecat [unresolved]: #unresolved-questions Naming the methods: while `try_set_high` and `try_toggle` seem ok, `try_is_set_high` name is a bit weird. + +Handling multiple error types in drivers. This RFC does not attempt to solve this issue, +leaving everything as it is. From 165b975533926151ac048fd145ee808a2d4e748a Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Mon, 18 Nov 2019 18:42:00 +0300 Subject: [PATCH 4/9] Modify the proposal to reflect the ryankurte's vision --- rfcs/0393-digital-v3.md | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/rfcs/0393-digital-v3.md b/rfcs/0393-digital-v3.md index f053c641..76b56c0e 100644 --- a/rfcs/0393-digital-v3.md +++ b/rfcs/0393-digital-v3.md @@ -49,23 +49,30 @@ produce an error due to the ambiguity of used trait. Due to this fact, one also This RFC proposes the following changes: -* Add `digital::v3` interface with the same traits as `digital::v2`, but with methods named with `try_` prefix. +* Add `digital::v3` with both fallible and infallible traits. +Fallible traits will provide the same interface as `digital::v2`, but will reside under `fallible` module and have +`try_` prefix for the trait methods. For example, `ToggleableOutputPin` will have `try_toggle` method instead of `toggle`. -* Provide transparent conversion from `digital::v2` traits to `digital::v3` traits. +Infallible traits will be exactly the same as in `digital::v1` interface. + +* Provide transparent conversion from `digital::v1` and `digital::v2` traits to `digital::v3` traits. * Change `digital::v1` compatibility layer to convert from `digital::v3` traits instead of `digital::v2` ones. * Add `V2OutputPin` proxy to the `digital::v2` compatibility layer to provide `digital::v3` -> `digital::v2` downgrade. -* Add `digital::v3` traits to prelude. -* Un-deprecate `digital::v1` traits, add a note for driver developers. -* Deprecate `digital::v2` traits in favor of `digital::v3` traits. -* Move HALs back to `digital::v1` interface. +* Deprecate `digital::v2` traits in favor of `digital::v3` fallible traits. +* Switch from `digital::v1` to `digital::v3` default. +* Move HALs to infallible `digital::v3` interface where applicable. +* Survey published driver crates and make a concerted effort to pull them all up to `digital::v3`. +* Drop `digital::v2` and `digital::v3` completely (with the semver-trick to previous versions if possible). ### Consequences +* Code that implements `digital::v1` traits will be transparently switched to `digital::v3` infallible traits +or (in case of explicit `digital::v1` usage) this compatibility will be provided via blanket implementations. +* Code that uses `digital::v1` traits will not require any changes provided that `digital::v3` traits are in scope. * End users will be able to use both fallible and infallible traits and import them via `embedded-hal` prelude. * HALs will export GPIOs as infallible (as they really are). * Driver developers can move to fallible `digital::v3` traits with minimal effort. -* End users that use drivers with `digital::v2` interface will continue using them without any changes in -most of the cases. Newer `digital::v3` objects could be passed to `digital::v2` consumers via `V2OutputPin` shim. +* For the first time both `digital::v1` and `digital::v2` interfaces can be used without any changes. # How We Teach This [how-we-teach-this]: #how-we-teach-this From ff4af71586bff19656ecd3a0fcfce2f1cd8fd509 Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Wed, 15 Jan 2020 12:14:58 +0300 Subject: [PATCH 5/9] Fix summary --- rfcs/0393-digital-v3.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0393-digital-v3.md b/rfcs/0393-digital-v3.md index 76b56c0e..22582cb8 100644 --- a/rfcs/0393-digital-v3.md +++ b/rfcs/0393-digital-v3.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -This RFC proposes `digital::v3` fallible interface and un-deprecation of `digital::v1` interface. +This RFC proposes `digital::v3` with both fallible and infallible interfaces and deprecation of the `digital::v2` interface. # Motivation [motivation]: #motivation From f92950ca8bd4055621b9fefe88eeee697d96ee95 Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Wed, 15 Jan 2020 12:16:08 +0300 Subject: [PATCH 6/9] Fix formatting --- rfcs/0393-digital-v3.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rfcs/0393-digital-v3.md b/rfcs/0393-digital-v3.md index 22582cb8..0fed585b 100644 --- a/rfcs/0393-digital-v3.md +++ b/rfcs/0393-digital-v3.md @@ -22,6 +22,7 @@ As a result, user code suffers from useless `.unwrap()` constructions. This RFC aims to solve these problems. Currently, `embedded-hal` digital traits are used from three perspectives: + * driver developers * hal developers * end-users @@ -29,7 +30,7 @@ Currently, `embedded-hal` digital traits are used from three perspectives: `digital::v2` traits were introduced to provide driver developers with a way to work with potentially fallible GPIO pins. Such pins can be created by drivers for external GPIO expanders connected via fallible peripherals (e.g. i2c). At the same time, most MCUs do not have fallible GPIOs and hals are not -required to implement fallible `digital::v2` interface for them. +required to implement fallible `digital::v2` interface for them. Due to the deprecation of `digital::v1` traits, hal maintainers were annoyed by deprecation warnings and switched to `digital::v2` traits to get rid of these warnings. This change introduced another problem: From 17f52bcc99ad6c240216eed627532e6e94af191a Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Wed, 15 Jan 2020 12:19:38 +0300 Subject: [PATCH 7/9] Fix naming details --- rfcs/0393-digital-v3.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rfcs/0393-digital-v3.md b/rfcs/0393-digital-v3.md index 0fed585b..145c96a7 100644 --- a/rfcs/0393-digital-v3.md +++ b/rfcs/0393-digital-v3.md @@ -51,9 +51,8 @@ produce an error due to the ambiguity of used trait. Due to this fact, one also This RFC proposes the following changes: * Add `digital::v3` with both fallible and infallible traits. -Fallible traits will provide the same interface as `digital::v2`, but will reside under `fallible` module and have -`try_` prefix for the trait methods. -For example, `ToggleableOutputPin` will have `try_toggle` method instead of `toggle`. +Fallible traits will provide the same interface as `digital::v2`, but will have Fallible prefix for the trait names and `try_` prefix for the trait methods. +For example, `ToggleableOutputPin` will become `FallibleToggleableOutputPin` with `try_toggle` method instead of `toggle`. Infallible traits will be exactly the same as in `digital::v1` interface. * Provide transparent conversion from `digital::v1` and `digital::v2` traits to `digital::v3` traits. From 58b5edd26c73871555e15a091264dfb1c825fbb4 Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Wed, 15 Jan 2020 12:25:17 +0300 Subject: [PATCH 8/9] Drop v1+v2 instead of v2+v3 --- rfcs/0393-digital-v3.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0393-digital-v3.md b/rfcs/0393-digital-v3.md index 145c96a7..7d3ae81b 100644 --- a/rfcs/0393-digital-v3.md +++ b/rfcs/0393-digital-v3.md @@ -62,7 +62,7 @@ Infallible traits will be exactly the same as in `digital::v1` interface. * Switch from `digital::v1` to `digital::v3` default. * Move HALs to infallible `digital::v3` interface where applicable. * Survey published driver crates and make a concerted effort to pull them all up to `digital::v3`. -* Drop `digital::v2` and `digital::v3` completely (with the semver-trick to previous versions if possible). +* Drop `digital::v1` and `digital::v2` completely (with the semver-trick to previous versions if possible). ### Consequences From b3548f033856c017eb0c140336fa3876e5a6e40a Mon Sep 17 00:00:00 2001 From: Vadim Kaushan Date: Wed, 15 Jan 2020 12:30:36 +0300 Subject: [PATCH 9/9] Add another alternative --- rfcs/0393-digital-v3.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rfcs/0393-digital-v3.md b/rfcs/0393-digital-v3.md index 7d3ae81b..2cd502bb 100644 --- a/rfcs/0393-digital-v3.md +++ b/rfcs/0393-digital-v3.md @@ -87,8 +87,10 @@ The proposed approach does not provide hal and driver developers with a warning # Alternatives [alternatives]: #alternatives -Changing `digital::v2` interface to provide different method names in all the pin traits. +* Changing `digital::v2` interface to provide different method names in all the pin traits. This solves the name clashing problem, but does not solve `digital::v1` deprecation problem. +* Releasing a new version of `embedded-hal` with just one set of (fallible+infallible) traits, +with `digital::{v1,v2}` modules removed. # Unresolved questions [unresolved]: #unresolved-questions