Skip to content
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

setting MODE1 options (ALLCALL) #15

Closed
edlins opened this issue Apr 25, 2018 · 5 comments
Closed

setting MODE1 options (ALLCALL) #15

edlins opened this issue Apr 25, 2018 · 5 comments

Comments

@edlins
Copy link
Owner

edlins commented Apr 25, 2018

MUST HAVE

  • Provide a mechanism for setting PCA9685 MODE1 ALLCALL bit.
  • Correct PCA9685_initPWM() to clear _PCA9685_ALLCALLBIT by default.

NICE TO HAVE

  • Include all options in MODE1 and MODE2.
  • Leverage existing bit definitions.

Right now, develop is hardcoded to set ALLCALL. It should change to clear ALLCALL. Add a new function int PCA9685_MODE1_opts(unsigned char) where the unsigned char represents the options in MODE1 as defined in PCA9685.h. PCA9685_initPWM(int, unsigned char, unsigned int) should use reasonable defaults when PCA9685_MODE1_opts() is not called. _PCA9685_AUTOINCBIT should always be set (that's half the point of the lib).

int ret = PCA9685_MODE1_opts(0x00 | _PCA9685_SUB1BIT | _PCA9685_ALLCALLBIT);
@edlins
Copy link
Owner Author

edlins commented Apr 25, 2018

Alternately this could be done with an extern unsigned char _PCA9685_MODE1_OPTIONS that is set directly from applications instead of adding a function.

@pvint
Copy link
Contributor

pvint commented Apr 27, 2018

int ret = PCA9685_MODE1_opts(0x00 | _PCA9685_SUB1BIT | _PCA9685_ALLCALLBIT);

I like this!
I would like the ability to use it to clear the AUTOINC bit if I so choose, however. (Although off the top of my head I cannot think of a situation where it would really be hurting anything)

I think I will look at this tonight, but if I don't get to it tonight it's highly unlikely that I will have time over the weekend so if you don't see anything tonight you won't see anything for a few days at least.

@pvint
Copy link
Contributor

pvint commented Apr 28, 2018

Added the PR #20 for this.

If there's anything you don't like about the implementation feel free to let me know (just don't dock my pay). :)

Edit: Just committed the change to remove setting the ALLCALL bit by default.

@edlins edlins mentioned this issue Apr 28, 2018
@edlins
Copy link
Owner Author

edlins commented May 1, 2018

Added PR #24 for my solution. Please test this before I merge into develop. Also LMK if you have any feedback on the code changes. Thanks!

@edlins
Copy link
Owner Author

edlins commented May 1, 2018

This got a little ugly with unrelated changes that I didn't branch out, but I'm confident so I merged it. If there's a problem, please open a new issue.

@edlins edlins closed this as completed May 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants