-
Notifications
You must be signed in to change notification settings - Fork 43
Add begin() method #27
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
Conversation
|
Hi, and thanks for the contribution 🙂 I'm not sure about this one though. As stated in the README
The usual I agree that the I2C address can be set either way with your PR, but it also means that you can create an instance and start using it without ever setting the address, which would be error prone. It also means that you can set this address using the constructor, and then use Moreover, most of the libs I've seen use What do you think ? 🙂 |
|
Hi. Obviously, I have a different take :-) As for
You're right, I missed that, I have fixed that now. Is this PR better suited for inclusion now? |
blemasle
left a comment
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.
Alright, you convinced me 🙂 Despite the fact that I still think the begin() pattern used by a lot of libraries is misleading and confusing, I get the "everything else works that way" argument.
Would you review the suggestions I made though ?
Cascade from `begin` with I2C address argument to `begin` w/o argument, it's clearer than calling `init()` from both places. Co-authored-by: Bertrand Lemasle <blemasle@gmail.com>
|
Thanks for the nice review feedback. |
It's usual for Arduino driver libs to have a constructor suitable for storage class static, and a
begin()method that's called fromsetup(). The I2C address can thus be set in either way.This PR adds that function.