4 ms·
The idea is good, but the code is very mediocre. The state machine has many redundant checks, very difficult to figure out how it works.
by SomeoneFromCA 6y ago
The idea is good, but the code is very mediocre.
The state machine has many redundant checks, very difficult to figure out how it works.
- ivanilves 6y agoThanks for your expertise, you are very welcome to propose any real improvements, if you can ;)
- SomeoneFromCA 6y agoSure, no problems, will give you feedback later, but here is one recommendation: combine group of booleans frequently used together in conditions into a new separate boolean variable. Say, for example, bool isMoving = (isOpening or isClosing).
- ivanilves 6y agoboolean grouping sounds neat, thanks!
- shoo 6y agoivanilves, thank you for writing this up and sharing the code & the video of the end result, it is a fun project! As a thought experiment I've had a play to see what the state machine logic might look like if we try to replace the bool state variables with a set of explicit finite states, and to push the state transition logic into a pure function. Results are here: https://gist.github.com/fcostin/851c1b4d1e3cb75ba972408151f185ba https://gist.github.com/fcostin/851c1b4d1e3cb75ba972408151f1... It is more verbose, but one advantage of structuring it like this (that i've only partially succeeded at) is that state transitions only happen at most once per each call to advance_state. Might make it easier to read through and follow the logic. It also makes the state transition logic easy to unit test, since it is pure-functional instead of being mixed together with sensing and doing actions (unsure how much of a concern or possibility unit testing is with ino files, I've never done any arduino dev myself). Not sure if this is anything like what SomeoneFromCA was thinking.
- ivanilves 6y agoThanks for your code, looks like this can help improving my state machine indeed. I'll try to apply that approach when I get my next slice of free time ;)