Add utility functions to access various part of source capabilities #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Even though the SourceCapabilities message is just a list of 32 bit pdos, some properties are encoded in the first pdo (aka the mandatory vsafe 5v one). Moving to a newtype allows adding some utility functions to get the whole message capabilities directly on it. Signed-off-by: Sjoerd Simons <[email protected]>
The voltage/current/power values in the various pdos require translations to real numbers; Add utility functions for this and rename the field to raw_ to avoid accidentally using the wrong function. For clarity also suffix the function by the returned unit if it's not the standard unit. Also for consistency use max instead of maximum everywhere Signed-off-by: Sjoerd Simons <[email protected]>
fmckeogh
reviewed
May 20, 2024
| } | ||
|
|
||
| impl SPRProgrammablePowerSupply { | ||
| pub fn max_voltage_mv(&self) -> u16 { |
There was a problem hiding this comment.
I understand this is subjective, but I'm not loving the _mv here. Which of the following do you think is best?
max_voltage() -> u16, with a doc comment explaining the return value is in millivoltsmax_voltage() -> MilliVoltswhereMilliVoltsis some type representing voltage units (I'm assuming there must be some crate with these types already)max_voltage_millivolts() -> u16
There was a problem hiding this comment.
There is uom which is meant to have no_std functionality and has types for e.g. voltage; So i can have a look at that to see if that ends up being overkill or note..
fwiw I discounted the first option is it ends up being surprising if voltage doesn't give you volts.. The third option is also ok by me, it's just a bit more typing (with mv being a common shorthand)
There was a problem hiding this comment.
I like uom, I'm happy to merge this commit as is, and I'll implement it.
fmckeogh
reviewed
May 20, 2024
| u16::from(self.raw_min_voltage()) * 100 | ||
| } | ||
|
|
||
| pub fn max_current_ma(&self) -> u16 { |
fmckeogh
reviewed
May 20, 2024
|
|
||
| impl EPRAdjustableVoltageSupply { | ||
| pub fn max_voltage_mv(&self) -> u16 { | ||
| u16::from(self.raw_max_voltage()) * 100 |
okhsunrog
added a commit
to okhsunrog/usb-pd-rs
that referenced
this pull request
Jun 18, 2024
Add utility functions to access various part of source capabilities
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.