Accelerometer FXLS8974 - bug in library ?

[adrian_chmiel]

Hi,

I work on your new product FXLS8974 accelerometer and probably I found bug.

It's regarding interrupt configuration, fuction fxls896xaf_config_interrupt with look:

...
 // configure the interrupts sources with desired pin configuration setting for fxls896xaf
status = sensor_comm_write(&pDriver->comHandle, FXLS896xAF_SENS_CONFIG4, 1, &ctrlReg);
// Enable the desired interrupt sources.
status = sensor_comm_write(&pDriver->comHandle, FXLS896xAF_INT_EN, 1, &pConfig->intSources.w);
// configure the interrupt routing
status = sensor_comm_write(&pDriver->comHandle, FXLS896xAF_INT_EN, 1, &pConfig->int1_2.w);

status = set_mode(pDriver, ACTIVE, NULL);
...

I think, in marked line, should be call to FXLS896xAF_INT_PIN_SEL, not again FXLS896xAF_INT_EN.

BTW
How to put the code in the frame? Like <code> in standard forums?

Best regards

Adrian Chmiel

[TomasVaverka · Accepted solution]

Hi Adrian,

You are right, the correct implementation is:

// configure the interrupt routing
status = sensor_comm_write(&pDriver->comHandle, FXLS896xAF_INT_PIN_SEL, 1, &pConfig->int1_2.w);

Thanks for pointing this out, we will fix it ASAP. 

The code can be inserted/edited using the Insert/Edit code sample button:

Best regards,

Tomas

[adrian_chmiel]

Hi Tomas,

I found another bug, a small one this time.

file fxls896xaf_driver.h, desctription of fxls896xaf_deinit() function (end of file)

/*! @brief       The interface function to de-initialize the fxls896xaf sensor.
 *  @details     This function de-initialize the fxls896xaf sensor.
 *  [in]   pDriver - Pointer to the driver
 *  [out]  selftest - Pointer to the self-test metadata.
 *  @return      returns the status of the operation.
 */
uint8_t fxls896xaf_deinit(fxls896xaf_driver_t *pDriver);

In description is information about param[out], but this argument does not exist in the function.

it was probably created in the process of copying.

And next one, fxls896xaf_config_interrupt() function, line 368:

...
	//Read the CRTL3 and preserve the existing configuration bits of the control registers other than interrupt configuration bits
	status = sensor_comm_read(&pDriver->comHandle, FXLS896xAF_SENS_CONFIG4, 1, &ctrlReg);

	// Update the Ctrl reg with polarity and open drain/push pull
	ctrlReg |= ((uint8_t)(pConfig->control.ipol | (pConfig->control.pp_od << FXLS896xAF_SENS_CONFIG4_INT_PP_OD_SHIFT)) & (FXLS896xAF_SENS_CONFIG4_INT_POL_MASK | FXLS896xAF_SENS_CONFIG4_INT_PP_OD_MASK));

	// configure the interrupts sources with desired pin configuration setting for fxls896xaf
...

incorrect implementation of the register bit change, thus the bits cannot be reset.

My proposition of implementation:

...
	// Update the Ctrl reg with polarity and open drain/push pull
	ctrlReg = (ctrlReg & ~(FXLS896xAF_SENS_CONFIG4_INT_POL_MASK | FXLS896xAF_SENS_CONFIG4_INT_PP_OD_MASK)) | ((uint8_t)(pConfig->control.ipol | (pConfig->control.pp_od << FXLS896xAF_SENS_CONFIG4_INT_PP_OD_SHIFT)));
...

One more question, I'm introducing modifications to the library, which in my opinion will become more universal, is there a place for sending changes or proposition of changes?

Best regards,

Adrian Chmiel

[TomasVaverka]

Hi Adrian,

Thanks again for reporting these bugs. There is a plan to fix them in both the FXLS896xAF and FXLS8974CF driver releases by end of April. 

Best regards,

Tomas

[adrian_chmiel]

if I can suggest anything else, I would like to suggest a separate function to start and stop the accelerometer.
In library is a set_mode function in the code but it is local.

The libraries assume that the accelerometer is always on, if we want to otherwise, we would have to manually control the registers or modify lib.

I think it would be good if the libraries were checking the status of the accelerometer (ACTIVE/STANDBY) and not always running it as in the above interrupt setting function.

[andrewneil]

@adrian_chmiel 

In library is a set_mode function in the code but it is local.

A fix for this has now been promised:

https://community.nxp.com/t5/Sensors/FXLS896xAF-library-why-is-set-mode-not-public/m-p/1789229/highl...