edlins / libPCA9685

superfast PCA9685 library for Debian platforms. developed on Raspbian on a Pi B+.
MIT License
13 stars 8 forks source link

Allow initialization of multiple devices #71

Closed pvint closed 5 years ago

pvint commented 5 years ago

Re #21

When attempting to initialize more than one device all devices on the bus were being reset when PCA9685_initPWM() was called. Perhaps there was a reason to do so, but it looks to me like it's fine to simply reset the desired device.

This change resets only the one device, and I have tested with two devices, one at 0x40 and one at 0x60 and all looks good in my testing.

I also added changes to examples/quickstart/quickstart.c to demonstrate this. (I added an option for a second device in seizure mode, but if you set the second address to 0x00 it ignores it)

Thanks to @deelerke for contacting me and giving me a bump on this!

deelerke commented 5 years ago

thanks @pvint, I tested this with three devices on address 40, 41 and 42 and they all work fine ! very nice

edlins commented 5 years ago

This looks okay. I love one line fixes. If quickstart was part of the core library, I would've written those changes differently, but it's fine for quickstart. I'll see if I can fix the Travis CI build..

edlins commented 5 years ago

Okay, when PCA9685_initPWM() was changed from

ret = _PCA9685_writeI2CRaw(fd, _PCA9685_GENCALLADDR, 1, &resetval);

to

ret = _PCA9685_writeI2CRaw(fd, addr, 1, &resetval);

The addr passed in to _PCA9685_writeI2CRaw() was changed from _PCA9685_GENCALL (value 0x00) to the actual addr (value 0x40). Consequently, the actual output of PCA9685test logged the actual addr value 0x40 instead of the original expected GENCALL value 0x00 and was flagged as a failure due to the diff:

2: +diff ../test/PCA9685_expected_output ./PCA9685_actual_output
2: 19c19
2: < _PCA9685_ioctl(): msg 0:   msg.addr = 0x00 msg.flags = 0x00 msg.len = 1 *msg.buf = 0x06 
2: ---
2: > _PCA9685_ioctl(): msg 0:   msg.addr = 0x40 msg.flags = 0x00 msg.len = 1 *msg.buf = 0x06 

So the PCA9685_expected_output file needs to be updated or regenerated after the lib change. The file in the test folder can be edited to match (not great) or regenerated by running

./test/PCA9685test -td 1 40 > PCA9685_actual_output

as executed by CMakeLists.txt and logged in the Travis CI build. If regenerated with the above command, rename PCA9685_actual_output to PCA9685_expected_output and move it in the test folder to replace the old expected output. If editing the expected output file manually, also fix the additional two diffs:

2: 64c64
2: < _PCA9685_ioctl(): msg 0:   msg.addr = 0x00 msg.flags = 0x00 msg.len = 1 *msg.buf = 0x06 
2: ---
2: > _PCA9685_ioctl(): msg 0:   msg.addr = 0x10 msg.flags = 0x00 msg.len = 1 *msg.buf = 0x06 
2: 112c112
2: < _PCA9685_ioctl(): msg 0:   msg.addr = 0x00 msg.flags = 0x00 msg.len = 1 *msg.buf = 0x06 
2: ---
2: > _PCA9685_ioctl(): msg 0:   msg.addr = 0x40 msg.flags = 0x00 msg.len = 1 *msg.buf = 0x06 

After the expected output file is fixed, the Travis CI build should pass.

edlins commented 5 years ago

In summary (sorry for the mess of comments):

  1. Fix comment in PCA9685_initPWM() in PCA9685.c
  2. Fix addr values in PCA9685_expected_output
  3. Fix PCA9685_initPWM documentation in README.md
  4. Add PCA9685.c to Changed under [0.8] in CHANGELOG.md
pvint commented 5 years ago

I'll try to do this this evening - Thanks Scott!

pvint commented 5 years ago

Did the updates - the Travis build is failing with something related to olaclient now.

That's all I have time for tonight - I can have a look at that tomorrow night.

Paul

edlins commented 5 years ago

Thanks for making the changes! It seems olaclient is not running correctly in the Travis environment. I commented out the check for the log file in the travis script for now so at least the build succeeds. But I need to fix this. I'll either merge this PR as-is and fix later, or try to fix in this PR. I'll look into it tomorrow.

pvint commented 5 years ago

No problem, thank you!

Just as an aside: I used this heavily in controlling all of my lighting on my boat all of last year, and the only problems I had were on a higher level (ie: my code that was using this library). IMO, it's been proven to be stable and reliable, and maybe time for a 1.0. :)