leka / LekaOS_Explorations

An exploration repo to use as a dirty stage before including in the main LekaOS
Apache License 2.0
2 stars 0 forks source link

LSM6DSOX - Get Configuration #10

Closed YannLocatelli closed 4 years ago

YannLocatelli commented 4 years ago

Le but est de sortir une version avec une simple connexion i2c et récupérer la configuration. La difficulté ici est d'utiliser le driver et de faire une bonne première structure.

YannLocatelli commented 4 years ago

Il manque l'intégration avec les nouvelles fonctions proposées par le driver situées dans le group Basic Configuration [Ligne 9474 à 11877]

YannLocatelli commented 4 years ago

Aussi, il y a une redondance dans l'utilisation de l'I2C entre l'accéléromètre et le gyroscope qui peut être allégé avec une nouvelle classe qui gèrera n'importe quelle interface I2C.

Exemple: Accelerometer [Ligne 58 à 99] avec Gyroscope [Ligne 58 à 99]

ladislas commented 4 years ago

Il manque l'intégration avec les nouvelles fonctions proposées par le driver situées dans le group Basic Configuration (ligne 9474)

@YannLocatelli tu peux mettre le lien?

ladislas commented 4 years ago

Aussi, il y a une redondance dans l'utilisation de l'I2C entre l'accéléromètre et le gyroscope qui peut être allégé avec une nouvelle classe qui gèrera n'importe quelle interface I2C.

Exemple: Accelerometer [Ligne 58 à 99 ] avec Gyroscope [Ligne 58 à 99 ]

@YannLocatelli il y a plusieurs solutions:

  1. soit faire une classe LKI2C ou SensorI2C qui hérite de mbed I2C et qui ajoute les deux méthodes (attention aux _address qui sera pas dispo et qu'il faudra avoir en paramètre
  2. soit avoir une interface SensorBase qui propose des virtual functions dans la déclaration et dont Accelerometer et Gyroscope héritent et qui peuvent les redéfinir en private (parce qu'on a pas besoin de l'appeler directement normalement).
  3. on doit même pouvoir avoir SensorBase qui définit les deux méthodes en private directement

Je testerai la 3. pour voir, ça me semble le mieux.

ladislas commented 4 years ago

Je testerai la 3. pour voir, ça me semble le mieux.

Quoi que... maintenant que j'écris ça, je suis pas sûr que d'avoir une class SensorBase face sens parce que ça voudrait dire prendre en compte tous les capteurs.

Dans le cas là, on voit qu'on utilise deux fois la même chose mais c'est parce que c'est le même composant.

Si on avait eux deux composants différents, on aurait peut être un en i2c et l'autre en spi, on se dirait pas qu'on a deux fois le même.

C'est du code qu'on va pas réécrire plusieurs fois et qui est hardware spécifique, ça me choque en fait pas qu'il soit dédoublé. On peut laisser pour le moment.

YannLocatelli commented 4 years ago

Le lien pour le group Basic Configuration a été mis à jour, on peut le retrouve ici aussi -> [Lien]

Pour ce qui est de l'I2C, la structure actuelle sera la même pour les composants en I2C de ST vus jusque là, de sûr ça concernera le LSM303AGR et le HTS221 à minima. Sans oublier qu'elle sera encore dédoublée pour les classes MLC et FSM du LSM6DSOX à venir.

C'est du code qu'on va pas réécrire plusieurs fois et qui est hardware spécifique, ça me choque en fait pas qu'il soit dédoublé. On peut laisser pour le moment.

Je reste sur ça pour le moment, ça sera un point à revoir si on redéfinie la structure.

ladislas commented 4 years ago

@YannLocatelli j'ai refait un passage ce matin au réveil après une grande tasse de café.

Il y a plusieurs choses qui me chiffonnent ;)

  1. la séparation de accéléromètre et gyroscope alors qu'ils devraient être dans la même classe LSM6DSOXSensor
  2. la duplication de beaucoup de code (due au 1.)
  3. une confusion sur le nom des choses par exemple status qui n'est pas vraiment un état
  4. le manque de parallélisme entre getter et setter qui crée de la confusion
  5. un manque d'encapsulation et de routines pour permettre une meilleure abstraction
  6. enfin, il y a une différence entre le nom de la PR et ce que fait le code

Tous ces points vont dans le sens du "on a été trop vite, on est pas parti dans le bon sens".

Heureusement, pour résoudre tout ça, il suffit de faire ce qu'on aurait du faire dès le début... 🥁

un schéma ! 🎉

Commençons par ça, un papier, un crayon, et un schema qui explique ce qu'on va faire et pourquoi on va le faire.

YannLocatelli commented 4 years ago

Je me permets de remettre ici ce que j'ai pu en partie dire par téléphone:

Proposition de solution [1]
Rédiger un document et/ou un wiki pour décrire les bonnes pratiques et les "erreurs" déjà commises. Il(s) peut(vent) compléter un document existant déjà: Règles de codage C++.

Proposition de solution [2] (discuté par téléphone)
La première étape est de réaliser un cahier des charges avec des diagrammes associés pour :

En attendant de répondre à ces demandes, je ferme cette PR.