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

LSM6DSOX - Get Configuration #10

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions LKExp-Sensors-LSM6DSOX_Examples/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,13 @@ Construction relies on examples provided by ST.
* [Driver](https://github.com/STMicroelectronics/STMems_Standard_C_drivers/tree/master/lsm6dsox_STdC/driver) -> commit `3fca83f875bbcbb6a428a7deb0ff01c0e5a2b033` (30 April 2020)
* [Examples](https://github.com/STMicroelectronics/STMems_Standard_C_drivers/tree/master/lsm6dsox_STdC/example)

## Abreviation
ladislas marked this conversation as resolved.
Show resolved Hide resolved

* FS : Full Scale
* FSM : Finite State Machine
* GY : Gyroscope
* ODR : Output Data Rate
* OIS : Optical Image Stabilization
* MLC : Machine Learning Core
* UI : User Interface
ladislas marked this conversation as resolved.
Show resolved Hide resolved
* XL : Accelerometer
134 changes: 123 additions & 11 deletions LKExp-Sensors-LSM6DSOX_Examples/lib/accelerometer/accelerometer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,19 @@ Accelerometer::Accelerometer(I2C *i2c, uint8_t address) : _i2c(i2c), _address(ad
*/
LSM6DSOXAccStatusTypeDef Accelerometer::init(void *init)
{
///* Output data rate selection - power down. */
//if (lsm6dsox_xl_data_rate_set(&_reg_ctx, LSM6DSOX_XL_ODR_OFF) != LSM6DSOX_OK)
//{
// return LSM6DSOX_ERROR;
//}

///* Full scale selection. */
//if (lsm6dsox_xl_full_scale_set(&_reg_ctx, LSM6DSOX_2g) != LSM6DSOX_OK)
//{
// return LSM6DSOX_ERROR;
//}
/* Initialize the device for driver usage */
if (lsm6dsox_init_set(&_reg_ctx, LSM6DSOX_DRV_RDY) != LSM6DSOX_Acc_OK)
{
return LSM6DSOX_Acc_ERROR;
}

/* Output data rate selection and full scale selection. */
_status.ui.xl.odr = _status.ui.xl.LSM6DSOX_XL_UI_52Hz_LP;
_status.ui.xl.fs = _status.ui.xl.LSM6DSOX_XL_UI_4g;
Comment on lines +48 to +49
Copy link
Member Author

@YannLocatelli YannLocatelli May 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

La forme est illisible, la faute à un enum anonyme imbriquée dans plusieurs struct anonymes. Le côté anonyme signifie qu'il n'y a pas de nom, ça n'empêche pas d'avoir un type.
On peut retrouver ladite structure ici.

Si il y a un moyen d'accéder au enum sans avoir à passer les autres struct je suis preneur.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

si tu parles de _status.ui.xl.odr je ne suis pas d'accord, pour le coup c'est ultra standard comme forme (on peut reprocher les abréviations pas claires mais c'est très spécifique), avec les .. En Swift on aura que ça.

Pour le coup je trouve que leur structure est assez propre.

De toute manière, ce genre de chose ne sera écrit qu'une fois et bien documenté. Après on en parlera plus.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je parlais plus de la partie droite de l'affectation : _status.ui.xl.LSM6DSOX_XL_UI_52Hz_LP

On peut voir une redondance des termes ui et xl et la valeur de l'enum ne peut être affecté qu'à partir d'un objet, ici _status.
Ca implique que laissé tel quel, pour passer en paramètre il faut entrer _status.ui.xl.LSM6DSOX_XL_UI_52Hz_LP plutôt que simplement LSM6DSOX_XL_UI_52Hz_LP

if (lsm6dsox_mode_set(&_reg_ctx, NULL, &_status) != LSM6DSOX_Acc_OK)
{
return LSM6DSOX_Acc_ERROR;
}

return LSM6DSOX_Acc_OK;
}
Expand All @@ -62,6 +64,116 @@ LSM6DSOXAccStatusTypeDef Accelerometer::read_id(uint8_t *id)
return LSM6DSOX_Acc_OK;
}

/**
* @brief Read component status
* @param powerMode is the power mode
* @param dataRate is output data range in float
* @param fullScale is full scal in hexadecimal value
* @retval 0 in case of success, an error code otherwise
*/
LSM6DSOXAccStatusTypeDef Accelerometer::get_status(PowerModeAcc *powerMode, float *dataRate, uint16_t *fullScale)
{
LSM6DSOXAccStatusTypeDef success = LSM6DSOX_Acc_OK;
if (lsm6dsox_mode_get(&_reg_ctx, NULL, &_status) != LSM6DSOX_Acc_OK)
{
return LSM6DSOX_Acc_ERROR;
}

switch((_status.ui.xl.odr & 0x0F))
ladislas marked this conversation as resolved.
Show resolved Hide resolved
{
case 0:
*dataRate = 0;
break;
case 1:
*dataRate = 12.5f;
break;
case 2:
*dataRate = 26.0f;
break;
case 3:
*dataRate = 52.0f;
break;
case 4:
*dataRate = 104.0f;
break;
case 5:
*dataRate = 208.0f;
break;
case 6:
*dataRate = 416.0f;
break;
case 7:
*dataRate = 833.0f;
break;
case 8:
*dataRate = 1667.0f;
break;
case 9:
*dataRate = 3333.0f;
break;
case 10:
*dataRate = 6667.0f;
break;
case 11:
*dataRate = 1.6f;
break;
default:
success = LSM6DSOX_Acc_ERROR;
break;
}
if (success == LSM6DSOX_Acc_ERROR)
{
return success;
}


*powerMode = (*dataRate == 0) ? Power_Off_Acc
ladislas marked this conversation as resolved.
Show resolved Hide resolved
: ((_status.ui.xl.odr & 0x20) >> 5) ? Ultra_Low_Power_Acc
: (not(_status.ui.xl.odr & 0x10) >> 4) ? High_Performance_Acc
: (*dataRate > 100.0f) ? Normal_Power_Acc
: Low_Power_Acc;


switch((_status.ui.xl.fs))
{
case 0:
*fullScale = 2;
break;
case 1:
*fullScale = 16;
break;
case 2:
*fullScale = 4;
break;
case 3:
*fullScale = 8;
break;
default:
success = LSM6DSOX_Acc_ERROR;
break;
}

return success;
}

/**
* @brief Read component interrupt status
* @param dataReady flag
* @retval 0 in case of success, an error code otherwise
*/
LSM6DSOXAccStatusTypeDef Accelerometer::get_int_status(uint8_t *dataReady)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

le nom de la fonction est pas clair et ce qu'elle fait ne sont pas clair pour moi.
tu dis get_int_status mais en même temps tu récupères des données en passant un pointeur.

Copy link
Member

@ladislas ladislas May 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

une autre manière de découper pourrait être la suivante:

bool dataAvailable() {
	if (lsm6dsox_all_sources_get(&_reg_ctx, &_int_status) == LSM6DSOX_Acc_OK) { // on check pour le positif, c'est plus naturel (en général)
		// et on return true si c'est ok
		return true;
	}
	else { // on garde le else pour bien montrer que c'est l'un ou l'autre.
		return false;
	}
}

// et plus tard dans le code

if (dataAvailable()) {
	auto ret = getAvailableData(&newData);
	if (ret == status::ok) {
		analyzeData(&newData);
	}
}

{
LSM6DSOXAccStatusTypeDef success = LSM6DSOX_Acc_OK;
if (lsm6dsox_all_sources_get(&_reg_ctx, &_int_status) != LSM6DSOX_Acc_OK)
{
return LSM6DSOX_Acc_ERROR;
}

*dataReady = _int_status.drdy_xl;

return success;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pourquoi définir un success plutôt que juste return LSM6DSOX_Acc_OK comme tu le fais déjà dans le if.

et surtout que tu dis que tu vas récupérer le status de l'interrupt (donc s'il y a des données dispos) mais qu'en fait tu get le status de l'accéléromètre au sens large.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tout à fait pour le success, ça a été corrigé dans le commit 1ec5825

tu dis que tu vas récupérer le status de l'interrupt (donc s'il y a des données dispos) mais qu'en fait tu get le status de l'accéléromètre au sens large.

C'est effectivement pas la bonne forme pour utiliser les interrupts. Quand j'ai voulu intégrer tous les status je me suis retrouvé avec la fonction all_sources_getet qui menait à la fin à tous les interrupts du composant. Dans le cas de l'accéléromètre il n'y en a qu'un qui lui est proprement utile.

}

int32_t LSM6DSOX_acc_io_write(void *handle, uint8_t WriteAddr, uint8_t *pBuffer, uint16_t nBytesToWrite)
{
return ((Accelerometer *)handle)->io_write(pBuffer, WriteAddr, nBytesToWrite);
Expand Down
14 changes: 14 additions & 0 deletions LKExp-Sensors-LSM6DSOX_Examples/lib/accelerometer/accelerometer.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ typedef enum
LSM6DSOX_Acc_ERROR = -1
} LSM6DSOXAccStatusTypeDef;

typedef enum
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

il faut préféré les enum class, voir ici: http://www.stroustrup.com/C++11FAQ.html#enum

C'est Bjarn lui même qui le dit ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ca a été ajouté dans le commit 34c9ea8

Même si ça allège le nom, les comparaison (avec !=) ne passe plus, je suis obligé de cast à chaque comparaison avec des éléments externes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tu peux donner un type à ton enum avec enum class: uint8_t par exemple.

testes et dis moi si ça marche :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ca a déjà été fait dans le commit 34c9ea8, comme on peut le voir ici : [Lien]

{
High_Performance_Acc = 0,
ladislas marked this conversation as resolved.
Show resolved Hide resolved
Normal_Power_Acc = 1,
Low_Power_Acc = 2,
Ultra_Low_Power_Acc = 3,
Power_Off_Acc = 4
}PowerModeAcc;

typedef enum
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pareil ici:

enum class AccTransmissionStatus {
	ok       =  0,
	error    = -1,
	overflow = -2
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

et d'ailleurs pourquoi -1 et -2 plutôt que 1 et 2?

0 c'est que tout est OK mais une raison particulière à aller dans le négatif?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L'idée d'affecter des enum pour les OK/ERROR vient du stm32duino. Et ils avaient basé leur ERROR à -1.

Par contre je doute que ça ait plus de sens d'aller dans les négatifs, donc on peut aller dans les positifs sans soucis si ça a un intérêt d'optimisation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l'intérêt de donner un numéro c'est qu'après on peut checker l'error avec try-block.

Si on a des errors spécifiques à nous, on peut faire un enum. sinon c'est bien aussi d'utiliser les exceptions de std:: https://en.cppreference.com/w/cpp/error/exception qui intègre overflow et runtime error par exemple.

{
Transmission_Acc_OK = 0,
Expand All @@ -47,6 +56,8 @@ class Accelerometer
Accelerometer(I2C *i2c, uint8_t address=LSM6DSOX_I2C_ADD_L);
LSM6DSOXAccStatusTypeDef init(void *init);
LSM6DSOXAccStatusTypeDef read_id(uint8_t *id);
LSM6DSOXAccStatusTypeDef get_status(PowerModeAcc *powerMode, float *dataRate, uint16_t *fullScale);
LSM6DSOXAccStatusTypeDef get_int_status(uint8_t *dataReady);

/**
* @brief Utility function to read data.
Expand Down Expand Up @@ -105,6 +116,9 @@ class Accelerometer
stmdev_ctx_t _reg_ctx;
static const unsigned int TEMP_BUF_SIZE = 32;

lsm6dsox_md_t _status;
lsm6dsox_all_sources_t _int_status;

};

#ifdef __cplusplus
Expand Down
135 changes: 124 additions & 11 deletions LKExp-Sensors-LSM6DSOX_Examples/lib/gyroscope/gyroscope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,19 @@ Gyroscope::Gyroscope(I2C *i2c, uint8_t address) : _i2c(i2c), _address(address)
*/
LSM6DSOXGyroStatusTypeDef Gyroscope::init(void *init)
{
///* Output data rate selection - power down. */
//if (lsm6dsox_gy_data_rate_set(&_reg_ctx, LSM6DSOX_GY_ODR_OFF) != LSM6DSOX_OK)
//{
// return LSM6DSOX_ERROR;
//}

///* Full scale selection. */
//if (lsm6dsox_gy_full_scale_set(&_reg_ctx, LSM6DSOX_2000dps) != LSM6DSOX_OK)
//{
// return LSM6DSOX_ERROR;
//}
/* Initialize the device for driver usage */
if (lsm6dsox_init_set(&_reg_ctx, LSM6DSOX_DRV_RDY) != LSM6DSOX_Gyro_OK)
{
return LSM6DSOX_Gyro_ERROR;
}

/* Output data rate selection and full scale selection. */
_status.ui.gy.odr = _status.ui.gy.LSM6DSOX_GY_UI_52Hz_LP;
_status.ui.gy.fs = _status.ui.gy.LSM6DSOX_GY_UI_500dps;
if (lsm6dsox_mode_set(&_reg_ctx, NULL, &_status) != LSM6DSOX_Gyro_OK)
{
return LSM6DSOX_Gyro_ERROR;
}

return LSM6DSOX_Gyro_OK;
}
Expand All @@ -62,6 +64,117 @@ LSM6DSOXGyroStatusTypeDef Gyroscope::read_id(uint8_t *id)
return LSM6DSOX_Gyro_OK;
}

/**
* @brief Read component status
* @param powerMode is the power mode
* @param dataRate is output data range in float
* @param fullScale is full scal in hexadecimal value
* @retval 0 in case of success, an error code otherwise
*/
LSM6DSOXGyroStatusTypeDef Gyroscope::get_status(PowerModeGyro *powerMode, float *dataRate, uint16_t *fullScale)
{
LSM6DSOXGyroStatusTypeDef success = LSM6DSOX_Gyro_OK;
if (lsm6dsox_mode_get(&_reg_ctx, NULL, &_status) != LSM6DSOX_Gyro_OK)
{
return LSM6DSOX_Gyro_ERROR;
}

switch((_status.ui.gy.odr & 0x0F))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

même remarque sur pourquoi et à quoi ça sert le &

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Le commentaire a été ajouté dans le commit 34c9ea8

{
case 0:
*dataRate = 0;
break;
case 1:
*dataRate = 12.5f;
break;
case 2:
*dataRate = 26.0f;
break;
case 3:
*dataRate = 52.0f;
break;
case 4:
*dataRate = 104.0f;
break;
case 5:
*dataRate = 208.0f;
break;
case 6:
*dataRate = 416.0f;
break;
case 7:
*dataRate = 833.0f;
break;
case 8:
*dataRate = 1667.0f;
break;
case 9:
*dataRate = 3333.0f;
break;
case 10:
*dataRate = 6667.0f;
break;
// case 11:
// *dataRate = 1.6f;
// break;
default:
success = LSM6DSOX_Gyro_ERROR;
break;
}
if (success == LSM6DSOX_Gyro_ERROR)
{
return success;
}


*powerMode = (*dataRate == 0) ? Power_Off_Gyro
: (not(_status.ui.gy.odr & 0x10) >> 4) ? High_Performance_Gyro
: (*dataRate > 100.0f) ? Normal_Power_Gyro
: Low_Power_Gyro;

switch((_status.ui.gy.fs))
{
case 0:
*fullScale = 250;
break;
case 1:
*fullScale = 125;
break;
case 2:
*fullScale = 500;
break;
case 4:
*fullScale = 1000;
break;
case 6:
*fullScale = 2000;
break;
default:
success = LSM6DSOX_Gyro_ERROR;
break;
}

return success;
}

/**
* @brief Read component interrupt status
* @param dataReady flag
* @retval 0 in case of success, an error code otherwise
*/
LSM6DSOXGyroStatusTypeDef Gyroscope::get_int_status(uint8_t *dataReady)
{
LSM6DSOXGyroStatusTypeDef success = LSM6DSOX_Gyro_OK;
if (lsm6dsox_all_sources_get(&_reg_ctx, &_int_status) != LSM6DSOX_Gyro_OK)
{
return LSM6DSOX_Gyro_ERROR;
}

*dataReady = _int_status.drdy_g;

return success;
}

int32_t LSM6DSOX_gyro_io_write(void *handle, uint8_t WriteAddr, uint8_t *pBuffer, uint16_t nBytesToWrite)
{
return ((Gyroscope *)handle)->io_write(pBuffer, WriteAddr, nBytesToWrite);
Expand Down
15 changes: 15 additions & 0 deletions LKExp-Sensors-LSM6DSOX_Examples/lib/gyroscope/gyroscope.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ typedef enum
LSM6DSOX_Gyro_ERROR = -1
} LSM6DSOXGyroStatusTypeDef;

typedef enum
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

même remarque pour enum class

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fait dans le commit 34c9ea8

{
High_Performance_Gyro = 0,
Normal_Power_Gyro = 1,
Low_Power_Gyro = 2,
// Ultra_Low_Power_Gyro = 3,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pas de ultra low power? comment c'est possible?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pas pour le gyroscope :) (cf. datasheet)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Power_Off_Gyro = 4
}PowerModeGyro;

typedef enum
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

même remarque pour enum class

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fait dans le commit 34c9ea8

{
Transmission_Gyro_OK = 0,
Expand All @@ -47,6 +56,9 @@ class Gyroscope
Gyroscope(I2C *i2c, uint8_t address=LSM6DSOX_I2C_ADD_L);
LSM6DSOXGyroStatusTypeDef init(void *init);
LSM6DSOXGyroStatusTypeDef read_id(uint8_t *id);
LSM6DSOXGyroStatusTypeDef get_status(PowerModeGyro *powerMode, float *dataRate, uint16_t *fullScale);
LSM6DSOXGyroStatusTypeDef get_int_status(uint8_t *dataReady);


/**
* @brief Utility function to read data.
Expand Down Expand Up @@ -105,6 +117,9 @@ class Gyroscope
stmdev_ctx_t _reg_ctx;
static const unsigned int TEMP_BUF_SIZE = 32;

lsm6dsox_md_t _status;
lsm6dsox_all_sources_t _int_status;

};

#ifdef __cplusplus
Expand Down
Loading