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 3 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/**
23 changes: 23 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,23 @@
# 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)

## 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
185 changes: 185 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,185 @@
/**
******************************************************************************
* @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
*/
LSM6DSOXAccStatusTypeDef Accelerometer::init(void *init)
{
/* 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 •
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) != LSM6DSOX_Acc_OK)
{
return LSM6DSOX_Acc_ERROR;
}

return LSM6DSOX_Acc_OK;
}

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

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 •
edited
Loading

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

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.

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

/* Prevent recursive inclusion -----------------------------------------------*/

#ifndef __ACCELEROMETER_H__
#define __ACCELEROMETER_H__

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

#include <assert.h>
#include "mbed.h"
#include "lsm6dsox_reg.h"

/* Defines -------------------------------------------------------------------*/

/* Typedefs ------------------------------------------------------------------*/

typedef enum
ladislas marked this conversation as resolved.
Show resolved Hide resolved
{
LSM6DSOX_Acc_OK = 0,
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,
Transmission_Acc_ERROR = -1,
Transmission_Acc_OVERFLOW = -2
} TransmissionAccStatusTypeDef;

/* Class Declaration ---------------------------------------------------------*/

/**
* Abstract class of an LSM6DSOX Inertial Measurement Unit (IMU) 6 axes
* sensor.
*/

class Accelerometer
{
public:
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.
* @param pBuffer: pointer to data to be read.
* @param RegisterAddr: specifies internal address register to be read.
* @param NumByteToRead: number of bytes to be read.
* @retval 0 if ok, an error code otherwise.
*/
uint8_t io_read(uint8_t* pBuffer, uint8_t RegisterAddr, uint16_t NumByteToRead)
{
//if (!_i2c) return Transmission_ERROR;

int ret;

/* Send device address, with no STOP condition */
ret = _i2c->write(_address, (const char*)&RegisterAddr, 1, true);
if(!ret) {
/* Read data, with STOP condition */
ret = _i2c->read(_address, (char*)pBuffer, NumByteToRead, false);
}

if(ret) return Transmission_Acc_ERROR;
return Transmission_Acc_OK;
}

/**
* @brief Utility function to write data.
* @param pBuffer: pointer to data to be written.
* @param RegisterAddr: specifies internal address register to be written.
* @param NumByteToWrite: number of bytes to write.
* @retval 0 if ok, an error code otherwise.
*/
uint8_t io_write(uint8_t* pBuffer, uint8_t RegisterAddr, uint16_t NumByteToWrite)
{
//if (!_i2c) return Transmission_ERROR;

int ret;
uint8_t tmp[TEMP_BUF_SIZE];

if(NumByteToWrite >= TEMP_BUF_SIZE) return Transmission_Acc_OVERFLOW;

/* First, send device address. Then, send data and STOP condition */
tmp[0] = RegisterAddr;
memcpy(tmp+1, pBuffer, NumByteToWrite);

ret = _i2c->write(_address, (const char*)tmp, NumByteToWrite+1, false);

if(ret) return Transmission_Acc_ERROR;
return Transmission_Acc_OK;
}

private:
I2C *_i2c;
uint8_t _address;

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
extern "C" {
#endif
int32_t LSM6DSOX_acc_io_write( void *handle, uint8_t WriteAddr, uint8_t *pBuffer, uint16_t nBytesToWrite );
int32_t LSM6DSOX_acc_io_read( void *handle, uint8_t ReadAddr, uint8_t *pBuffer, uint16_t nBytesToRead );
#ifdef __cplusplus
}
#endif

#endif // __ACCELEROMETER_H__
Loading