Traumflug
Configtool refactoring
Let's open a new issue here, comments on commits get lost easily.
Part of the previous discussion: https://github.com/Traumflug/Teacup_Firmware/commit/09bcb0cf4234fbdd8e8b02bcb6e32de0b053d5e4#commitcomment-17734637
I just moved branch fixpins to experimental, this part appears to be solved.
Then also removed branch fixpins-old, which had turned out to replace some user #defines by the default ones.
On branch configtool-refactoring I separated change for distinct list for booleans into its own commit. Then rebased all this onto fixpins and made sure the changes there were applied to the new Board and Printer classes, too (had to apply that manually). Then moved the other commits as far as I couldn't find it working to experimental, too.
Great advances, so far! Thanks for the code, @phord.
Thinking of these remaining commits I plan to get Configtool regression tests working, first. Then add tests for the stuff which was achieved the last few days:
- recovery from missing boolean #defines,
- recovery from missing value #defines,
- recovery from value #defines which are defined as boolean ones.
Right now the commit introducing the --save can't run this command:
./configtool.py --load=config/board.3drag.h --save=build/test/board.3drag.h --quit
Well, Morgen ist auch noch ein Tag :-) ("tomorrow is as well a day", means: one shouldn't try everything at once)
Everything is pushed, don't forget to fetch with --prune, then you see what I see.
Got these Configtool regression tests working and moved stuff up to there onto experimental. Some of the commits left on configtool-refactoring don't pass the regression test, others make me wondering what the goal of the journey is.
To some extents we have a MMVC strategy now. One M for a configuration loaded by --load, the other M for the configuration held in the GUI. As far as I can see, if GUI code messes up one of the items, these new regession tests won't catch it. Configurations loaded via command line don't end up in the GUI and vice versa. It smells a bit like a coding exercise instead of an attempt to make Configtool more reliable and easier to use for users trying hard to be dumb.
Just removed the older "integrity" test after finding the new one to catch missing #defines just fine. This part works fine. Excellent!
Part of my goal for the ConfigTool CLI was to build regression tests for little use cases, like this, maybe:
./configtool.py --load=config/printer.mendel.h --set ACCELERATION_REPRAP=true --set ACCELERATION_RAMPING=false --save=build/test/printer.mendel.h --build --quit
Any acceleration except ACCELERATION_RAMPING has been broken for more than a year, it turns out. It would be nice to test individual settings like this. But probably this method is too complicated and instead we could add tests like this:
// config/printer.acceleration_reprap.h
#include "config/printer.mendel.h"
#undef ACCELERATION_RAMPING
#define ACCELERATION_REPRAP
It seems like there are too many combinations of options to catch them all with individually curated files, so a text file of interesting combinations to test could be used instead and then automated with the configtool CLI.
Test-should-succeed: ACCELERATION_REPRAP
ACCELERATION_RAMPING=false
ACCELERATION_REPRAP=true
Test-should-succeed: ACCELERATION_NONE
ACCELERATION_RAMPING=false
ACCELERATION_REPRAP=false
Test-should-fail: X_STEP_PIN has bogus pin
X_STEP_PIN=INVALID_PIN
Such file of test definitions could be parsed and tested using the ConfigTool CLI once I have --set implemented, for example.
Another test I'm interested in is to verify the thermistortables.h file gets generated sanely for various reasonable values of thermistors. This would be easily done with some hand-built testcases, except the CLI still should be used to generate the thermistortables.h.
As for GUI testing of the ConfigTool, I have ideas to push the capabilities further to let the CLI drive some GUI actions, further separating the M from the V. But I'm not sure how well those can be used, and I wasn't as interested in them for now.
Part of my goal for the ConfigTool CLI was to build regression tests for little use cases, like this, maybe: ...
Now I see it! This makes a lot of sense, of course. I agree that the second strategy, adding a few lines to config.h, is much easier to implement.
Regarding GUI I could imagine to let --load load files into the GUI and --save fetch values from there, but that looks like a step backwards regarding the MVC model at the first glance. Still it would verify the GUI layers for regressions and it's not too much work.
I plan to teach the GUI to avoid loading files again which were already loaded from the CLI. The CLI should also learn to load 'config.h', using the existing GUI code, of course. More V->M code movement. A full GUI exercise could be achieved with some primitive command parser that knows how to select named tabs, named menu items, etc. But I haven't investigated that very deeply, as I said earlier.