[WIP] First design to abstract I2C comm behind a class #4

azerupi · 2017-02-01T16:29:07Z

Le but est de créer une abstraction autour de la communication I2C pour chaque module. Un module est (généralement) une partie spécifique du robot commandé par une arduino, e.g. les capteurs ultrasons.

JonathanPetit · 2017-02-03T09:00:38Z

Le dernier commit est l'ajout du code arduino pour les capteurs ultrasons. Il y a encore un ou deux bugs dans le résultat de communication arrivé à la raspberry. Cela provient surement du code arduino. Ce sera fixer dans le courant de la semaine prochaine!

charlesvdv

Juste quelques détails à modifier pour que ce soit parfait !

charlesvdv · 2017-02-03T16:15:32Z

code/arduino/us_sensors.ino

+
+const byte SLAVE_ADDRESS = 0x04;
+const byte N_SENSORS = 2;
+const byte N_MEASURES = 5;


Utilise #define au lieu de const pour des constantes définies au dessus de ton code.

Personnellement je préfère les constantes qu'utiliser le préprocesseur. Il n'y à pas d'avantage à utiliser #define au lieu de const

http://arduino.stackexchange.com/a/510/8755

Je n'ai pas pensé que les bonnes pratiques du C sont différentes des bonnes pratiques du C++. C'est nickel alors!

charlesvdv · 2017-02-03T16:22:50Z

code/raspberrypi/range_sensors.py

+        self.n = self.get_number_of_sensors()
+
+    def get_range(self, sensor):
+        cmd = I2C.pack8(1, sensor)


Mettre les int dans des constantes pourrait être plus clair pour le lecteur.

Le mieux serait probablement d'utiliser une énumération

C'est vrai que ce serait plus propre avec des enums. Ca serait quand meme pas mal de faire juste pour avoir une vue d'ensemble sur les commandes qu'on peut faire et directement avoir l'id de la commande.

charlesvdv · 2017-02-03T16:27:17Z

code/raspberrypi/main.py

@@ -0,0 +1,11 @@
+from range_sensors import RangeSensor


Pas sur que ce code devrait être merger vu qu'on utilisera d'office pas comme ça dans la version finale.

Le main.py?
Oui, la c'est majoritairement pour pouvoir tester, mais je pense que ça peut mal de le merger. Il changera quand on ajoutera plus de code.

Ok pour le garder pour l'instant 😄 c'est vrai que ca ne fait pas de mal.

charlesvdv · 2017-02-05T17:32:13Z

code/arduino/us_sensors.ino

+
+  return sum / N_MEASURES;
+}
+


useless lines.

Tu parles des "blank lines"?

azerupi · 2017-02-08T19:37:11Z

J'ai résolu le problème qu'on avait !!

I2C

Monsieur Marchand m'a expliqué que dans le protocole I2C, le master demande à écrire et lire des données sur des registres spécifique d'un esclave. Pour lire plusieurs bytes d'affilées on doit indiquer un registre à partir duquel on va lire, tant que le maitre n'envoie pas de signal d’arrêt, l'esclave va envoyer les registres suivants.

Donc, pour résoudre le problème qu'on avait, j'ai ajouté une variable dans l'arduino qui mémorise l'état de la réponse à la commande. Quand on reçoit une commande de la Pi, on passe dans l'état command_processed = false et on ignore les autres messages que la Pi envoie, jusqu'au moment ou l'arduino à pu envoyé sa réponse.

Mais... je ne suis pas super content de cette approche, je pense que ce serait mieux de se mettre d'accord d'ignorer tous les bytes null 0x00. De cette façon, on ne dépends plus de l'ordre des messages. Tout byte non-null sera considéré une nouvelle commande, les bytes null seront ignorés ou considérés comme la fin d'une commande.

Qu'en pensez-vous?

Commandes

Après réflexion, je me dis que les commandes choisies ici ne sont pas toutes utiles.
Depuis qu'on a ajouté la commande get_number_of_sensors, la commande enumerate_sensors n'apporte plus rien de plus. Comme les capteurs sont numérotés de 1 à nombre de capteurs + 1, cette commande renverra juste une liste consécutive...

Je propose de la supprimer.

Abstraction arduino

J'ai quelques idées à explorer pour créer des abstractions pour le code Arduino, mais comme le code ici fonctionne déjà, je propose qu'on le merge déjà pour que tout le monde y ai accès facilement. Si je trouve un bon design je ferai un nouveau PR.

Donc je pense que ceci est prêt à être reviewed, @charlesvdv?

charlesvdv · 2017-02-09T08:44:54Z

code/raspberrypi/i2c.py

+            self.bus.write_byte(self.adress, data)
+
+    def receive(self, num_bytes=1):
+        if num_bytes == 1:


Normalement on a plus besoin de ca vu que read_i2c_block_data devrait aussi gérer les receive d'un byte et ca permetterait aussi d'etre plus conforme avec la nouvelle facon de faire les choses c-à-d mettre 0x00 sur le bus quand on a fini la commande.

charlesvdv · 2017-02-09T08:46:14Z

code/raspberrypi/i2c.py

+
+class I2C:
+
+    def __init__(self, adress):


C'est address et pas adress. Il y a plusieurs endroits ou il faut le changer (dans us_sensors.ino également).

charlesvdv · 2017-02-09T08:49:17Z

code/arduino/us_sensors.ino

+
+const byte SLAVE_ADDRESS = 0x04;
+const byte N_SENSORS = 2;
+const byte N_MEASURES = 5;


Je n'ai pas pensé que les bonnes pratiques du C sont différentes des bonnes pratiques du C++. C'est nickel alors!

charlesvdv · 2017-02-09T08:51:04Z

code/raspberrypi/main.py

@@ -0,0 +1,11 @@
+from range_sensors import RangeSensor


Ok pour le garder pour l'instant 😄 c'est vrai que ca ne fait pas de mal.

charlesvdv · 2017-02-09T08:57:25Z

code/raspberrypi/range_sensors.py

+        self.n = self.get_number_of_sensors()
+
+    def get_range(self, sensor):
+        cmd = I2C.pack8(1, sensor)


C'est vrai que ce serait plus propre avec des enums. Ca serait quand meme pas mal de faire juste pour avoir une vue d'ensemble sur les commandes qu'on peut faire et directement avoir l'id de la commande.

charlesvdv · 2017-02-09T08:59:49Z

code/arduino/us_sensors.ino

+        // Enumerate all the sensors
+        case 3:
+            for(byte i = 0; i < N_SENSORS; i++) {
+                Wire.write( i );


Gros gros détail mais tant qu'on y ait autant avoir la meme convention de codage partout. J'écrirais plutot Wire.write(i);

charlesvdv · 2017-02-09T09:00:03Z

code/arduino/us_sensors.ino

+            break;
+
+        case 4:
+            Wire.write( N_SENSORS );


La meme que juste au dessus.

charlesvdv · 2017-02-09T09:10:54Z

Un autre truc que l'on devrait ajouter sont des logs dans le code de la raspberry. J'ai oublié de mettre ca dans la review.

azerupi · 2017-02-09T09:14:32Z

Merci!
Je suis d'accord avec tout ce que tu as dis, je ferai les modifs tantôt.
Le logging est pas indispensable pour cette PR, il peut être ajouté par une PR ultérieurement :)

azerupi · 2017-02-09T15:46:28Z

Ok, je pense que j'ai adressé tout ce qu'il restait. J'ai également rajouté des commentaires, surtout dans le code python et j'ai supprimé la commande qui permet d'énumérer les capteurs, vu qu'elle n'as pas vraiment d'utilité.

A priori, je n'ai pas changé grand chose, mais sachez que je n'ai pas testé ce dernier commit.

JonathanPetit · 2017-02-09T16:28:48Z

Normalement vu ce que tu as changé cela ne doit pas poser trop de problèmes...
Mais je serais d'avis de le tester avant de le merge?

charlesvdv · 2017-02-09T19:20:47Z

code/raspberrypi/range_sensors.py

+class RangeSensor(I2C):
+    """
+    This class is an abstraction around the I2C communication with
+    the range-sensor mdoule.


typo avec module. @azerupi

charlesvdv · 2017-02-09T19:21:54Z

code/raspberrypi/range_sensors.py

+
+    Details of the "protocol" used:
+
+    The Raspberry Pi sens a byte to the module containing a command


ca serait plutot send non ?

Oui, bien vu!

charlesvdv · 2017-02-09T19:24:41Z

@azerupi dès que le code est tester et que tu as corrigé les 2 petites typos on pourra merger tout ca 😄

azerupi · 2017-02-10T10:38:58Z

J'ai testé, résolu quelques erreurs et ça fonctionne bien.

Il y à juste un truc qui peut être amélioré, c'est que l'arduino block pendant un petit moment quand le capteur pointe dans le vide et fini par renvoyer 0 comme distance. On pourrait essayer de résoudre le problème nous même, ou alors on pourrait regarder si en utilisant la librairie NewPing le problème disparaît de lui même.

En attendant, le code est bon pour merger. Je ferrai des tests avec NewPing quand je testerai mes idées d'abstractions pour l'I2C du code Arduino.

charlesvdv · 2017-02-10T13:26:52Z

Je pense que si on mets un timeout plus faible pour pulseIn on diminuerait le problème.

azerupi · 2017-02-10T14:52:52Z

J'ai essayé, mais ça n'avait pas l'air de changer quelque chose

charlesvdv · 2017-02-10T15:11:36Z

Ok parfait ! Je vais merger alors ! Merci beaucoup @azerupi @JonathanPetit pour la PR!

First design to abstract I2C comm behind a class

87656c1

azerupi added the I2C label Feb 2, 2017

azerupi and others added 2 commits February 2, 2017 17:43

Improve I2C abstraction

44f622c

[WIP] first arduino code for ultrason capteur

ea821dd

azerupi mentioned this pull request Feb 4, 2017

Interface I2C capteurs ultrasons #5

Open

charlesvdv reviewed Feb 5, 2017

View reviewed changes

charlesvdv mentioned this pull request Feb 7, 2017

Structure du programme raspi #7

Open

8 tasks

Resolve bugs and format arduino code

585c75e

charlesvdv requested changes Feb 9, 2017

View reviewed changes

Address feedback and add comments

f496a30

charlesvdv reviewed Feb 9, 2017

View reviewed changes

charlesvdv approved these changes Feb 9, 2017

View reviewed changes

azerupi added 2 commits February 9, 2017 21:04

Fix typos

793a0f2

Fix errors in the python code

27afa63

charlesvdv merged commit ae722e7 into Ecam-Eurobot:master Feb 10, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] First design to abstract I2C comm behind a class #4

[WIP] First design to abstract I2C comm behind a class #4

azerupi commented Feb 1, 2017

JonathanPetit commented Feb 3, 2017

charlesvdv left a comment

charlesvdv Feb 3, 2017

azerupi Feb 5, 2017 •

edited

Loading

charlesvdv Feb 9, 2017

charlesvdv Feb 3, 2017

azerupi Feb 5, 2017

charlesvdv Feb 9, 2017

charlesvdv Feb 3, 2017

azerupi Feb 5, 2017

charlesvdv Feb 9, 2017

charlesvdv Feb 5, 2017

azerupi Feb 5, 2017

azerupi commented Feb 8, 2017

charlesvdv Feb 9, 2017

charlesvdv Feb 9, 2017

charlesvdv Feb 9, 2017

charlesvdv Feb 9, 2017

charlesvdv Feb 9, 2017

charlesvdv Feb 9, 2017

charlesvdv Feb 9, 2017

charlesvdv commented Feb 9, 2017

azerupi commented Feb 9, 2017

azerupi commented Feb 9, 2017

JonathanPetit commented Feb 9, 2017

charlesvdv Feb 9, 2017 •

edited

Loading

charlesvdv Feb 9, 2017

azerupi Feb 9, 2017

charlesvdv commented Feb 9, 2017

azerupi commented Feb 10, 2017

charlesvdv commented Feb 10, 2017

azerupi commented Feb 10, 2017

charlesvdv commented Feb 10, 2017


		Details of the "protocol" used:

		The Raspberry Pi sens a byte to the module containing a command

[WIP] First design to abstract I2C comm behind a class #4

[WIP] First design to abstract I2C comm behind a class #4

Conversation

azerupi commented Feb 1, 2017

JonathanPetit commented Feb 3, 2017

charlesvdv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azerupi Feb 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azerupi commented Feb 8, 2017

I2C

Commandes

Abstraction arduino

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charlesvdv commented Feb 9, 2017

azerupi commented Feb 9, 2017

azerupi commented Feb 9, 2017

JonathanPetit commented Feb 9, 2017

charlesvdv Feb 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charlesvdv commented Feb 9, 2017

azerupi commented Feb 10, 2017

charlesvdv commented Feb 10, 2017

azerupi commented Feb 10, 2017

charlesvdv commented Feb 10, 2017

azerupi Feb 5, 2017 •

edited

Loading

charlesvdv Feb 9, 2017 •

edited

Loading