blahlicus / animus-family

keyboard firmware family for arduino compatible atmega u series microcontrollers
Apache License 2.0
75 stars 21 forks source link

animus 3 - comms mode 7 #29

Closed BlackCapCoder closed 5 years ago

BlackCapCoder commented 5 years ago

Mode 7 is used to update some variables in EEPROM:

https://github.com/blahlicus/animus-family/blob/aa2277483e1cb3ca81124d253ad17a8fd1db7bd4/animus-3/source/core/Comms.cpp#L132-L152

Firstly, if some of the 4 calls to Serial.available evaluates to false then we would update the wrong variables, or not update anything at all. Consider something like this:

printf '\0\1\2\3\7' > /dev/ttyACM0 # handshake
printf '\2\1\1\x20' > /dev/ttyACM0 # new data

It makes sense to split the logic that deals with the handshake from the one that deals with writing data, but this separation of logic often carries overhead - be it function call overhead or spinning up an entirely new program as in the case of bash -, which might delay the payload enough to miss align the variables.

Secondly, in ModI2CHost mode 7 means to dump the guests EEPROM:

https://github.com/blahlicus/animus-family/blob/aa2277483e1cb3ca81124d253ad17a8fd1db7bd4/animus-3/source/core/ModI2CHost.cpp#L237-L251

Shouldn't this instead request that the guest also update its variables?