[riot-notifications] [RIOT-OS/RIOT] drivers: support for NXP PCA9685 I2C 16-channel, 12-bit PWM controller (#10556)

Gunar Schorcht notifications at github.com
Fri Sep 6 18:38:19 CEST 2019

gschorcht commented on this pull request.

> +    return _read(dev, PCA9685_REG_MODE1, &byte, 1);
+static int _init(pca9685_t *dev)
+    /* set Auto-Increment flag */
+    EXEC_RET(_update(dev, PCA9685_REG_MODE1, PCA9685_MODE1_AI, 1));
+    /* switch off all channels */
+    EXEC_RET(_write_word(dev, PCA9685_REG_ALL_LED_OFF, PCA9685_ALL_LED_OFF));
+    /* set Auto-Increment flag */
+    uint8_t byte = 0;
+    _set_reg_bit(&byte, PCA9685_MODE2_OUTDRV, dev->params.inv);
+    _set_reg_bit(&byte, PCA9685_MODE2_OUTDRV, dev->params.out_drv);
+    _set_reg_bit(&byte, PCA9685_MODE2_OUTNE, dev->params.out_ne);

Hm, this executed only once. The while loop is not a big thing. I would prefer to leave it as it is, for the sake of clarity. We could think about not to define only a bit mask but also a bit shift value for each for register field. Another way would be to define structures for all registers.

But, since `_set_reg_bit` is only used during `_init`, there should be no performance problem.

But, I realized a real real problem.
-  _set_reg_bit(&byte, PCA9685_MODE2_OUTDRV, dev->params.inv);
+  _set_reg_bit(&byte, PCA9685_MODE2_INVERT, dev->params.inv);

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190906/c9aae12c/attachment-0001.htm>

More information about the notifications mailing list