Add serial control #34
Conversation
dsferruzza
commented
May 31, 2020
•
dsferruzza
commented
- do actual effects when receiving control message
- check that nothing breaks too much when storming the serial with correct messages
- check that nothing breaks too much when storming the serial with random bytes
- check that messages with incorrect CRC are discarded
- add ACK telemetry messages
- handle every setting
- send ACK messages when updating settings with motherboard buttons
dsferruzza
force-pushed
the
add-serial-control
branch
2 times, most recently
from
426c34c to
acd951f
Compare
dsferruzza
force-pushed
the
add-serial-control
branch
from
acd951f to
8192ea6
Compare
| m_cyclesPerMinuteCommand = cpm; | ||
| } | ||
|
|
||
| #if HARDWARE_VERSION == 2 || HARDWARE_VERSION == 3 |
There was a problem hiding this comment.
plutot que d'avoir des combinatoires de flag, il vaudrait surement mieux avoir des flags par features et un include hardwarev2 ou hardwarev3 qui les set pour être sûr que l'on oublie pas un flag par erreur
There was a problem hiding this comment.
ici c'est typiquement un flag has_telemetry by uart qui sera def sur hardwarev2 et v3 et pas sur v1
There was a problem hiding this comment.
Pas bête ; mais ça appelle des changements sur tout le reste de la base de code pour adopter ce système de manière uniforme. Je serai d'avis de faire ça indépendamment de cette PR.
There was a problem hiding this comment.
Très bonne idée @melf. Par contre comme @dsferruzza le suggère je ferais plutôt ça dans une autre PR.
| int time = millis(); | ||
|
|
||
| // We need to ensure we received the whole message | ||
| while (((time + 2) >= millis()) && (Serial6.available() >= 11)) { |
There was a problem hiding this comment.
humm, j'aime pas cette boucle, surtout avec des continue à l'intérieur. comment on peut s'assurer de sortir du while en cas de spam de l'uart ?
Cette boucle peut générer une dérive temporelle non bornée et pourquoi time+2 >millis ? si la boucle est indispensable et que la condition temporelle l'est aussi, peut-on l'écrire de manière à l'invalider à un moment ?
There was a problem hiding this comment.
J'ai mis une boucle ici pour être capable de gérer le fait qu'il pourrait y avoir autre chose que des messages qui soient reçus, par exemple des octets isolés. Par ailleurs, je voulais pouvoir prendre en compte rapidement plusieurs messages qui seraient envoyés à la suite. L'objectif c'est que ça soit le plus réactif possible (notamment à renvoyer un message ACK à la UI) sans impacter la respiration.
- on arrête d'essayer de traiter des messages si jamais :
- on a moins de 11 octets en buffer (= on est sûr qu'on n'a aucune chance d'avoir le message en entier donc on verra plus tard si on a reçu plus d'octets)
- ou si on a déjà passé plus de 2 ms à essayer (valeur arbitraire, pour éviter de prendre de la dérive temporelle sur les fonctions de respiration)
- les
continuepermettent de réessayer de traiter des messages si on a eu une erreur (mauvais CRC, mauvaise structure, pas les bons entêtes) pour être plus réactif que si on attend juste le tick d'après ; ce "retry" est limité dans le temps par les 2 critères évoqués ci-dessus.
Mon objectif ici est de tester ça sur un proto pour s'assurer que la respiration continue à fonctionner même dans le cas d'un RPi malveillant. J'ai déjà écrit du code pour essayer de flooder le MCU : makers-for-life/makair-telemetry#12
Tu penses que cette approche est pétée ?
| @@ -615,6 +615,58 @@ void sendAlarmTrap(uint16_t centileValue, | |||
| #endif | |||
| } | |||
|
|
|||
| // cppcheck-suppress unusedFunction | |||
| void sendControlAck(uint8_t setting, uint16_t valueValue) { | |||
| #if HARDWARE_VERSION == 1 | |||
There was a problem hiding this comment.
autre exemple de flag par feature : essayons d'éviter au max les #if #elif pour garder un code clair et maintenable
