Prepare I2C module for adding slave mode #241
Merged
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
I'm working on adding support for I2C slave mode. A type parameter to identify which mode the peripheral is in will be added to `Enabled` (as opposed to `I2C`; the mode is only important if the peripheral is enabled, and anything else would make implementing methods that are always available, if the peripheral is enabled, more complex than need be). I think with this new mode parameter in the mix, it is no longer obviously better to have a default value for the state parameter. With default type parameters, there's always a trade-off between brevity and convenience on one side, and clarity on the other. While master mode will certainly be more common, I think the brevity/convenience gained by using it as the default will not be worth the loss of clarity. This is not a clear-cut decision and I don't feel strongly about it. I'm open to changing this and making enabled/master the new default.
I'm about to add another module for I2C slave mode which is going to need access to `Error`. Leaving it were it is right now would introduce a circular dependency.
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.
I'm currently working on I2C slave mode. Since that might still take a bit more time (it's currently not working) and since it looks like I might be adding more error handling code on the master side to debug it, I figured I'd submit this now, to at least get some of it reviewed, and to keep any merging and rebasing on my side to a minimum.