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.

Sign up for GitHub

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

LSM6DSOX - Get Configuration #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc8edca
Initial commit
YannLocatelli May 15, 2020
016c3cc
Add i2c interface and get id of lsm6dsox
YannLocatelli May 15, 2020
3cbe9ef
Get device status and add abbreviations
YannLocatelli May 19, 2020
1ec5825
Data read and fix in init
YannLocatelli May 19, 2020
34c9ea8
Some precisions
YannLocatelli May 19, 2020
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
4 changes: 4 additions & 0 deletions LKExp-Sensors-LSM6DSOX_Examples/.mbed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
MBED_OS_DIR=./lib/mbed-os
TARGET=disco_f769ni
TOOLCHAIN=GCC_ARM
ROOT=.
24 changes: 24 additions & 0 deletions LKExp-Sensors-LSM6DSOX_Examples/.mbedignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
### Ignored directories to speed up compilation time

test/**
lib/mbed-os/components/**
lib/mbed-os/features/cellular/**
lib/mbed-os/features/cryptocell/**
lib/mbed-os/features/deprecated_warnings/**

lib/mbed-os/features/FEATURE_BLE/**
lib/mbed-os/features/FEATURE_BOOTLOADER/**

lib/mbed-os/features/lorawan/**
lib/mbed-os/features/lwipstack/**

lib/mbed-os/features/nanostack/**
lib/mbed-os/features/netsocket/**
lib/mbed-os/features/nfc/**
lib/mbed-os/features/unsupported/**

### Needed directories

# lib/mbed-os/features/device_key/**
# lib/mbed-os/features/frameworks/**
# lib/mbed-os/features/mbedtls/**
25 changes: 25 additions & 0 deletions LKExp-Sensors-LSM6DSOX_Examples/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# LKExp - LSM6DSOX Examples

## Goal

Create an abstraction class of lsm6dsox driver for Leka applications.
Construction relies on examples provided by ST.

## Ressources
[STMems\_Standard\_C\_drivers](https://github.com/STMicroelectronics/STMems_Standard_C_drivers) for:

* [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)

[LSM6DSOX datasheet](https://www.st.com/resource/en/datasheet/lsm6dsox.pdf)

## Abbreviation

* 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
251 changes: 251 additions & 0 deletions LKExp-Sensors-LSM6DSOX_Examples/lib/accelerometer/accelerometer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
/**
******************************************************************************
* @file accelerometer.h
* @author Yann Locatelli
* @brief Implement accelerometer of LSM6DSOX for Leka.
******************************************************************************
*/

/* Includes ------------------------------------------------------------------*/

#include "accelerometer.h"

/* Class Implementation ------------------------------------------------------*/

/** Constructor
* @param i2c object which handles the I2C peripheral
* @param address the address of the component
*/
Accelerometer::Accelerometer(I2C *i2c, uint8_t address) : _i2c(i2c), _address(address)
{
assert (i2c);
Copy link
Member

Choose a reason for hiding this comment

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

il faudra bien vérifier qu'on compile après avec NDEBUG https://en.cppreference.com/w/cpp/error/assert

voir aussi si c'est conseillé en dehors des tests de mettre des assert dans le code. pas sûr que ce soit utile de crasher le robot si l'i2c ne marche plus. on voudrait plutôt continuer en faisant remonter le problème.


_reg_ctx.write_reg = LSM6DSOX_acc_io_write;
_reg_ctx.read_reg = LSM6DSOX_acc_io_read;
_reg_ctx.handle = (void *)this;
}

/**
* @brief Initializing the component
* @param init pointer to device specific initalization structure
* @retval 0 in case of success, an error code otherwise
*/
AccStatus Accelerometer::init(void *init)
{
/* Initialize the device for driver usage */
if (lsm6dsox_init_set(&_reg_ctx, LSM6DSOX_DRV_RDY) != (int32_t)AccStatus::OK)
{
return AccStatus::ERROR;
}

//Get current status
if (lsm6dsox_mode_get(&_reg_ctx, NULL, &_status) != (int32_t)AccStatus::OK)
{
return AccStatus::ERROR;
}

/* Replace 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 •
edited
Loading

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) != (int32_t)AccStatus::OK)
{
return AccStatus::ERROR;
}

return AccStatus::OK;
}

/**
* @brief Read component ID
* @param id the WHO_AM_I value
* @retval 0 in case of success, an error code otherwise
*/
AccStatus Accelerometer::read_id(uint8_t *id)
{
if (lsm6dsox_device_id_get(&_reg_ctx, id) != (int32_t)AccStatus::OK)
{
return AccStatus::ERROR;
}

return AccStatus::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
*/
AccStatus Accelerometer::get_status(AccPowerMode *powerMode, float *dataRate, uint16_t *fullScale)
{
AccStatus ret = AccStatus::OK;
if (lsm6dsox_mode_get(&_reg_ctx, NULL, &_status) != (int32_t)AccStatus::OK)
{
return AccStatus::ERROR;
Comment on lines +83 to +85
Copy link
Member Author

Choose a reason for hiding this comment

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

La fonction proposée dans le driver récupère TOUTE la configuration, contrairement à la version sous mbed qui configure UN PAR UN chaque paramètre.

La raison pour laquelle il n'y a pas de set_status est dû au choix à faire entre ces deux manières (configurer tous les paramètres en une fois ou un par un).

Il faut aussi prendre en compte un commentaire fait plus haut qui implique un accès lourd aux valeurs (enum) de la configuration.

Copy link
Member

Choose a reason for hiding this comment

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

je pense que le problème vient surtout du design et du manque d'encapsulation.

ta class Accelerometer devrait avoir en private member:

  • soit une struct config
  • soit des membres séparés
// Soit une struct - plus propre je trouve
struct AccelerometerConfig {
	AccPowerMode powerMode,
	float dataRate,
	uint16_t scale,
}

class LSM6DSOXAccelerometer { // ⚠️ je pense que c'est important de renommer les classes actuelles avec le prefix `LSM6DSOX` pour bien montrer le `tight coupling`

public:

	// ... code

	// Soit tu get la config au total dans une struct bien définie
	void getConfig(AccelerometerConfig * config);
	// Soit tu get les données individuelles
	// tu peux d'ailleurs faire les deux si besoin, c'est pas l'un ou l'autre
	AccPowerMode getPowerMode();
	float getDataRate();
	uint16_t getScale();

	// et tu peux faire pareil avec le set
	void setConfig(AccelerometerConfig * config) {
		_config = * config;
	}
	void setPowerMode(AccPowerMode mode) {
		// Soit
		_config.powerMode = mode;
		// ou
		_powermode = mode;
	}

private:
	// Soit une struct - plus propre je trouve
	Config _config;

	// Soit des membres séparés
	AccPowerMode _powerMode,
	float _dataRate,
	uint16_t _scale,

Copy link
Member Author

Choose a reason for hiding this comment

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

En fait, _status est le _config de ton exemple et la struct lsm6dsox_md_t [Lien vers sa définition] qui correspond à la struct Config dans ton exemple...

La question porte plus sur l'utilisation d'une telle struct ou de tout découper. On est favorable à une struct plutôt que des membres séparés, mais soit :

  • on utilise la struct lsm6dsox_md_t proposé par le driver avec l'inconvénient mentionné un peu plus haut,
  • on réécrit une struct (exemple Config) pour d'une part palier au soucis du point précédent et d'autre part ajouter des paramètres supplémentaires tel que le PowerMode.

Bien que la seconde nécessite plus de travail et de code, il permettra d'avoir un code plus clair.

}

switch((_status.ui.xl.odr & 0x0F)) //Value of odr is 0xYZ with Y the power mode and Z an enum frequence
{
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:
ret = AccStatus::ERROR;
break;
}
Comment on lines +88 to +129
Copy link
Member Author

Choose a reason for hiding this comment

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

Il n'existe pas de struct/enum pour associer la fréquence à un float. Seulement est disponible un identifiant en hexadecimal qui est reconvertis ici.

Il faudra donc prévoir une fonction ou un dictionnaire (map en C++) pour convertir dans les deux sens plus facilement, accessible de partout puisque ces paramètres seront souvent réutilisés.

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 me pointer dans la doc où tout ça est décrit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Au niveau du code on retrouve leur définition ici : [Lien]
Dans la datasheet, tu peux le retrouver en Table 51 (p.56)

Copy link
Member

Choose a reason for hiding this comment

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

ah je comprends ! en fait tu veux fair ressortir le data rate dans un format qu'on peut lire?

if (ret == AccStatus::ERROR)
{
return ret;
}

//Value of odr is 0xYZ with Y the power mode and Z an enum frequence
if (*dataRate == 0) {
*powerMode = AccPowerMode::OFF;
} else {
if ((_status.ui.xl.odr & 0x20) >> 5) {
*powerMode = AccPowerMode::ULTRA_LOW;
} else if ((_status.ui.gy.odr & 0x10) >> 4) {
if(*dataRate < 100.0f){
*powerMode = AccPowerMode::LOW;
} else {
*powerMode = AccPowerMode::NORMAL;
}
} else {
*powerMode = AccPowerMode::HIGH_PERFORMANCE;
}
}

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:
ret = AccStatus::ERROR;
break;
}

return ret;
}

/**
* @brief Read component interrupt status
* @param dataReady flag
* @retval 0 in case of success, an error code otherwise
*/
AccStatus Accelerometer::get_int_status(uint8_t *dataReady)
{
if (lsm6dsox_all_sources_get(&_reg_ctx, &_int_status) != (int32_t)AccStatus::OK)
{
return AccStatus::ERROR;
}
*dataReady = _int_status.drdy_xl;

return AccStatus::OK;
}

/**
* @brief Read component data
* @param mg_X data value of acceleration on X axis in mg
* @param mg_Y data value of acceleration on Y axis in mg
* @param mg_Z data value of acceleration on Z axis in mg
* @retval 0 in case of success, an error code otherwise
*/
AccStatus Accelerometer::get_data(float *mg_X, float *mg_Y, float *mg_Z)
{
//if (lsm6dsox_data_get(&_reg_ctx, NULL, &_status, &_data) != AccStatus::OK) //Lot of mistakes in the driver's function
//{
// return AccStatus::ERROR;
//}
//*mg_X = _data.ui.xl.mg[0];
//*mg_Y = _data.ui.xl.mg[1];
//*mg_Z = _data.ui.xl.mg[2];
Comment on lines +199 to +205
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'utilisation de la fonction lsm6dsox_data_get() a été abandonnée, il y a encore trop d'erreur dans cette fonction.
Je ferrai remonté les erreurs en temps libre.

Copy link
Member

Choose a reason for hiding this comment

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

curieux de voir les erreurs! :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed : STMicroelectronics/STMems_Standard_C_drivers#62 :)


AccStatus ret = AccStatus::OK;
uint8_t data_raw[6];

/* Read raw data values. */
if (lsm6dsox_acceleration_raw_get(&_reg_ctx, data_raw) != (int32_t)AccStatus::OK)
{
return AccStatus::ERROR;
}

float_t (* pConversionFunction) (int16_t raw_value);
switch(_status.ui.xl.fs)
{
case 0:
pConversionFunction = &lsm6dsox_from_fs2_to_mg;
break;
case 1:
pConversionFunction = &lsm6dsox_from_fs16_to_mg;
break;
case 2:
pConversionFunction = &lsm6dsox_from_fs4_to_mg;
break;
case 3:
pConversionFunction = &lsm6dsox_from_fs8_to_mg;
break;
default:
ret = AccStatus::ERROR;
break;
}

*mg_X = (float)(* pConversionFunction)((uint16_t)((uint16_t)data_raw[1] << 8) | data_raw[0]);
*mg_Y = (float)(* pConversionFunction)((uint16_t)((uint16_t)data_raw[3] << 8) | data_raw[2]);
*mg_Z = (float)(* pConversionFunction)((uint16_t)((uint16_t)data_raw[5] << 8) | data_raw[4]);

return ret;
}

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);
}

int32_t LSM6DSOX_acc_io_read(void *handle, uint8_t ReadAddr, uint8_t *pBuffer, uint16_t nBytesToRead)
{
return ((Accelerometer *)handle)->io_read(pBuffer, ReadAddr, nBytesToRead);
}
Comment on lines +243 to +251
Copy link
Member Author

Choose a reason for hiding this comment

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

Mentionné dans un appel, il serait intéressant de rentrer cette fonction à l'intérieur de la classe afin de simplifier son utilisation.

Loading