That’s interesting that using atof does not trigger this. I had your fix printing the command when it detected the wrong number of arguments and it looked just like a command that cut off in the middle, looking like a communication issue. Maybe atol causes some strange memory corruption in the UART FIFO or command String?
We need to switch to atof until this is addressed on the python side but also implement your check for the number of arguments provided because that is a good idea in general. I’ll do some more testing to confirm your observations re: atol causing the crash (SWD is connected to the pi’s gpio, so we could figure out the exact cause, but it’s probably not that important as we are using atol incorrectly anyway).
Thank you for the merge request, I’ll try to review it tonight.
At some point we really need to write some integration tests, I have some functional tests (in functional testing branch, a bit out of date now but I reproduced this bug with a modified version of one of them) for the firmware and I did testing with pySangaboard module, but I didn’t test the DeltaStage module enough. I’ve been thinking about an automated testing system for the Sangaboard (with encoders and photodiodes) for a while, this might be a good time to actually build it and also support testing of the full OFM software stack to avoid issues like this.
Fantastic work, thanks for figuring this out! I did wonder if sending floats was the issue, but dismissed it because surely then it would fail every time. I never thought there could be a nasty intermittent issue that pops up very occasionally for float moves.
Unless I’m very much mistaken, moves must be integers anyway (because they are in half-steps, and the drivers can’t make smaller steps than that) so the Python module definitely needs updating. IIRC there’s not a lot of checking done on the arguments in the Arduino code (at least not with my argument parsing - can’t remember if Filip replaced that) so it might also be possible to specify that arguments must be integers - shouldn’t be too hard to verify that all arguments are -?[\d]+, even if there isn’t a handy regexp parser…