-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Pcf8591 analog out #1285
Pcf8591 analog out #1285
Conversation
LGTM, but needs tests. I might just write the test myself to ease back into work (I've been away for 8 weeks). Let me know if you want to write the tests or if I should. Here's where they'd go: https://github.com/rwaldron/johnny-five/blob/master/test/expander.js#L1982-L2156 |
Is this really a good idea? The PCF8591 has a D/A converter that converts digital values to their corresponding analog voltage. The analog voltage can be more or less any voltage. It's not the same as PWM where the output voltage is either 0V or VDD where VDD is typically 5V. Although the technique used by this PR will almost certainly work, reusing the PWM mode for DAC will likely result in confusion. For example, it's easy to set the brightness of an LED using PWM. Setting the brightness of an LED using DAC is more complex. If the PWM mode is reused for DAC, Johnny-Five will think that the pin supports PWM, but it doesn't. |
I'm planning to add DAC support to Firmata (at the protocol level) at some point: firmata/protocol#32. |
@soundanalogous That sounds like a better way to go to me. |
This brings up another interesting question however which is how do we handle pin numbering with expander ICs? |
OK lets wait for soundanalogous for proper implementation. Then I can use his framework to rewrite my patch. |
@fivdi thanks for intervening with that info @soundanalogous re: pin numbering on ICs, if I understand your question correctly (which I might not), I generally stick to "as seen on the pcb" I agree that once a "dacWrite" (or whatever) is designed, then this can follow. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
The fix will have to wait on DAC support in firmata so I'm closing this PR. I've added the feature request to the Requests Features wiki page and linked back to this PR. |
Added possibility for PCF8591 to output analog value. Needs code review - see comments in code.