adafruit / Adafruit_APDS9960

Arduino drivers for APDS9960 gesture sensor
Other
37 stars 45 forks source link

Incorrect bit field definitions #26

Closed mark255bits closed 3 years ago

mark255bits commented 3 years ago

Hello,

The Adafruit APDS_9960 library is unable to configures some sensor register incorrectly as some bit fields are incorrectly defined. For example in Adafruit_APDS9960.h

/** Proxmity gain settings */
typedef enum {
  APDS9960_PGAIN_1X = 0x00, /**< 1x gain */
  APDS9960_PGAIN_2X = 0x04, /**< 2x gain */
  APDS9960_PGAIN_4X = 0x08, /**< 4x gain */
  APDS9960_PGAIN_8X = 0x0C  /**< 8x gain */
} apds9960PGain_t;

These values are already bit shifted as the PGAIN bit field uses the bits (3:2) of the register Control register One(0x8F). But if for example one were to use the method setProxGain this will not correct configure as the struct _control get() function bit shiftes again the pgain bit field. uint8_t get() { return (LDRIVE << 6) | (PGAIN << 2) | AGAIN; } A possible solution could be change the enum definition to:

/** Proxmity gain settings */
typedef enum {
  APDS9960_PGAIN_1X = 0x00, /**< 1x gain */
  APDS9960_PGAIN_2X = 0x01, /**< 2x gain */
  APDS9960_PGAIN_4X = 0x02, /**< 4x gain */
  APDS9960_PGAIN_8X = 0x03  /**< 8x gain */
} apds9960PGain_t;

Which will solve the problem and be more alligned bit the APDS99600 datasheet as they define the bit field values without bit shifting. image

The same bug is applicable to apds9960PPulseLen_t , apds9960LedDrive_t and apds9960LedBoost_t. The values of the different enums should be 0x00, 0x01x 0x02 and 0x03.

ladyada commented 3 years ago

hi - can you submit a PR? we'll review it :)

mark255bits commented 3 years ago

Ok, I will try to do it.

ladyada commented 3 years ago

thank you!