-
Notifications
You must be signed in to change notification settings - Fork 10
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
Remove DynamicOneShot
trait.
#19
Conversation
fe3a67a
to
78aafb3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I fixed the CI now, could you rebase the PRs?
78aafb3
to
bd95c9c
Compare
@eldruin, rebased the PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I added some comments.
@@ -235,7 +235,7 @@ impl BitFlags { | |||
} | |||
|
|||
pub mod channel; | |||
pub use channel::{ChannelId, ChannelSelection}; | |||
pub use channel::ChannelId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think we should expose ChannelSelection
publicly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't.
#[doc = $doc] | ||
$CH, | ||
)+ | ||
mod private { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you move this module inside this macro?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way the ChannelId
is sealed by making ChannelSelection
private.
|
||
/// Marker type for an ADC input channel. | ||
pub trait ChannelId<T>: private::Sealed { | ||
pub trait ChannelId<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should this trait not be sealed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is actually sealed since the ChannelSelection
type is private.
d796952
to
dad963e
Compare
dad963e
to
4f46a6e
Compare
@eldruin, can you have another look here? Thanks. |
@eldruin, please review this again. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay @reitermarkus. Thank you for your work.
From reading #10 (comment), it looks like adding this trait didn't actually solve the issue.
Depending on the device, this also allows reading invalid channels.