-
Notifications
You must be signed in to change notification settings - Fork 126
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
Merged in tjanson's work and overloaded write/read methods with options object #37
base: master
Are you sure you want to change the base?
Conversation
Uses pi-gpioutil, a wrapper for Wiring Pi's gpio utility, rather than quick2wire's gpio-admin. Benefits include more robust pin exports. Further improvement option: Speed up read/writes by using native code bindings, such as: https://github.com/eugeneware/wiring-pi
deduplicated auto-exporting code
rm getMode
test for board revision
README: inital example adapted README: linebreaks README: export example
I’ve implemented the same functionality (among other things) in #27 (I called it "automatic export", see the README of that branch). Which hasn’t been merged yet – probably because no one’s had the time to test it thoroughly. That doesn’t mean that we shouldn’t merge yours in the meantime though. One thing though: You changed the README and removed all its contents – instead of that, you should document the additional functions in the existing README. |
Thanks for the heads up tjanson. I'll reup the README, double check the code again, then we can merge. My thoughts are having a separate method, keeping the original write method as is, because of the additional overhead. For time sensitive applications such as robotics this could prove worth it. What are your thoughts? Think there really isn't an appreciable performance hit in the "automatic export" vs straight write? And will there be cases when a developer may not want auto open every time a write is called? |
If I remember correctly (this was ~8 months ago), the automatic export was optional, enabled by a passing a boolean flag to the write method – so, effectively, the same thing as having a separate, new method; giving the programmer full control in the choice of comfort vs speed. |
+1 for overloading the existing .write method. Not a huge fan of different functions with nearly the same functionality. The overhead will only be of the 'if' statement that checks whether the 'autoOpen' (or whatever) argument is true or not. That'll be a negligible delay compared to the delay that Linux itself introduces. The subsequent performance lag of having to check the pin's state is opt-in, so it should be ok I guess. That said, I don't think it should be a boolean argument, because it's very hard to tell by looking at the caller what the boolean is for. ie., Any suggestions for an alternative function signature? Considering consistency with other functions, I can only come up with Or am I all wrong, and it's just simpler to have a boolean argument? :) |
I hear you. Wrapping it into one function makes more sense. And once again, I agree that passing a boolean is frustrating and not at all self documenting. However, I would also find it frustrating having to wrap value as a key within an object, when I assume most people most the times will just want to just pass the basic: pin and value (and maybe cb). How about keeping the format intact as it is now: .write(pinNumber, 1, cb). But if the third parameter is an object vs a function then parse that for further options. Then it would like: .write(pinNumber, 1, {autoOpen: true}, cb). This also leaves plenty of room for further expansion. |
I'm in favor of merging tjansons work. I'll do some testing today of his branch, see if anything breaks. |
tjanson's branch seems to be working so far on Model B+, Revision: 1.2... |
I’ve created a rather simple method called writeRegardless (open to name suggestions here) that allows for writing to a pin without knowing whether its opened and set to the ‘out' direction. The method simply opens the pin if not opened already and sets the direction to out.
My argument for adding this is that there are legitimate cases where a script won’t know before hand which pins will be used. And I believe the logic to detect whether open or not should be scoped to the pi-gpio library, not the script referencing it.
I’d be curious to hear other’s remarks about this. I do understand that there is overhead with running testing logic, thats why it wouldn’t be a replacement for write, but more of a convenience method in the particular cases where it makes sense.