With respect, I’ve been trying to read the code but the present arrangement makes that very difficult. The majority of what I’ve suggested are improvements to readability, not performance. Of course performance is more important, but if I cannot follow the code, I cannot help.
Tasks, are separated into functions, technically true in that what you have is a computer program. But everything seems to be the names of things, often very generic things, not the action they perform. So the code is not self documenting and it is also poorly commented. There seems to be very little concept of user interface.
Thus my request for a high level overview. I was hoping that would help reduce the need for me to ask 1000 questions. Those that wrote the code understand the flow because it was in their head prior to being written. The rest of us don’t have that advantage.
I’ll give a few examples from cnc_ctrl_v1.ino
Right off the bat, good to see comments, wish they were more useful.
Left and right are not axis. An axis is a fixed reference line, usually orthogonal to other axis. These change length and angle constantly and are right angles to nothing. A ‘position’ on this ‘axis’ does not map to any point on the workpiece, it can change any time a different axis changes(chain wrap). It is not a position, it is a length, mostly represented by an encoder count. Wait, encoders are a different file, or 2. The only pseudo fixed property is the origin. These are vectors… no, because the setup is only about pins, this must be a hardware driver. No, even the definitions for direction are defined elsewhere. System? really? couldn’t get any more generic. Are they motors? No, motors are elsewhere. Motion, no. Kinematics, no. Is it a motorgearboxencoder whatever the heck that is? No. It has PID, values, is it a PID controller? No, that is elsewhere. What the heck is an axis?
So many names for different files describing essentially the same thing, but none of them really capture what is inside. There is no clear distinction between hardware and the theoretical model. This is a big problem because now you cannot change one without breaking the other. Moving on.
Serial.print(F(“PCB v1.”));
Serial.print(getPCBVersion());
Oh, print the version, then get it. Novel approach.
if (TLE5206 == true) { Serial.print(F(" TLE5206 “)); }
Serial.println(F(” Detected"));
What kind of value does TLE5206 represent? What type? Anything nonzero is true. What file do I need to look in to find out? Doesn’t matter, it is always " Detected" true or not.
sys.inchesToMMConversion = 1;
really? That had to be just here? What does it mean? There are 25.4mm per inch, not 1.
// Set initial desired position of the machine to its current position
leftAxis.write(leftAxis.read());
rightAxis.write(rightAxis.read());
zAxis.write(zAxis.read());
The comment shows human knowledge that the desired(commanded) position is not necessarily the position the machine is at. The statements imply that the machine makes no such distinction.
reportStatusMessage(STATUS_OK);
Now thats optimism. Couldn’t possibly be any other way. Could it? Do you think maybe we should check? No, it couldn’t possibly be in setup as the result of a brownout reset or serial monitor opening and all the motors are still screaming away. What is even the point of reporting status if it isn’t checked. By the way, why are status and state not defined together?
I understand that what exists is the result of a messy historical process. The scope of the project grew and artifacts were left behind. That happens when you are really on to something. I am not looking to remove any optimizations, just package them, so we can also provide optimizations for different hardware that the end user may have. I’ll just say it, I am not building a default maslow. I have different ideas, but need the firmware to evolve to accommodate it. I am not good enough to do it myself, so I’m pushing a little.
If I had to start from scratch, the way I described is how I would start. So as I slowly learn what the code is doing, I’ll cut and paste into that framework.
To be fair, I have to admit that after writing this post I discovered a lot more comments than I saw initially. Because I was looking for a high level overview, I was focused mostly on the header files. Looking for an API basically. Turns out most of the comments are in .cpp files which I just skimmed because I wanted to put pieces together, not see how each piece worked. So, if this post sounds sarcastic, sorry. My sarcasm setting sticks sometimes. I am aware that my frustration is mostly of my own manufacture. I should probably just delete, but I spent too much time typing to let it disappear.