This repository was archived by the owner on Oct 18, 2022. It is now read-only.
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.
Adding support for the SPI stack chips #10
base: master
Are you sure you want to change the base?
Adding support for the SPI stack chips #10
Changes from 21 commits
06d9b55
08197ad
cbd5159
233effe
e28690d
e07ac1d
ceda1d6
4bfdf97
a3422c2
7a4755a
7e41934
8a518a0
e82af20
a2e816e
fd9b3bc
fa2488a
a6d2db2
816d9f1
8127c02
33834ca
4e3b729
e72f625
eebfde2
a446e71
1474850
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
These can just
#[derive(Debug)]
, there shouldn't be any harm in thatThere 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.
You can also make them
enum
s instead ofstruct
s since they're just used as type markersThere 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.
Please add documentation to this type that explains what the type parameters are for
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.
DIE
could also be bounded by anActiveDie
trait, which would be implemented byDie0
andDie1
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.
No,
DIE
is actually supposed to be a type state, it doesn`t have to fulfill any trait bounds it is just there so rust knows which switch_die implementation it is supposed to use.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.
Sure. If you bound it by a trait that is only implemented by the 2 types that are supposed to be used though, users wouldn't be able to name invalid
Flash
types likeFlash<Bla, Bla, ()>
.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 mean technically the user is just supposed to _ the type state as the constructor only produces Die0 types, so if a user starts to mess around with the state parameter he is most likely doing something wrong isn't he?