Teacup_Firmware icon indicating copy to clipboard operation
Teacup_Firmware copied to clipboard

Some ConfigTool options should be code-driven

Open phord opened this issue 9 years ago • 1 comments

From @Traumflug

One thing I'm a bit unsure about is, how should we deal with Configtool? It's pretty obvious that people like you and me and @phord happily hack away on the firmware, but have or see little gain in adjusting Configtool, too. We put our extra #define into config.h somewhere and are done. On the other end of the spectrum we have users which barely manage to start a Python application. Those won't make Configtool more complete either. Configtool is a great thing, but it tends to fall behind.

From @phord

I think it's a good policy to require developers to update ConfigTool to match their code changes. But in practice I think it's ok to make that a complaining policy. Such as, "Thanks for adding this cool new feature. Please add it to configtool so we can accept it into the mainline." And then later, "Ok, I understand ConfigTool is obtuse and you don't speak python. I've added it for you here. Please review and tell me if I misunderstood."

But what I would really prefer is for ConfigTool to learn its options from the header files. It already learns the help text there. I agree it would be overkill to reinvent a GUI description language inside comments in the config headers. Some things would never make sense, such as the adjustPinVisibility function, and we would always tweak them in python. But the DISPLAY_TYPE options seem trivial to "learn" from board.generic.h. It might help us remember to keep the Doxygen current, too, if Doxygen-blocks was where it learned it from.

From @Traumflug

I hope the idea of Configtool is more than just adding some GUI-sugar on top of headers. A tool with just 1:1 relationships could be done easier. Configtool can do much more than just flipping switches, it also has the power to do things graphically (think of endstop locations, build room size defined by dragging them in a drawing), to do calculations (think of steps/mm calculator, thermistor table creation) or to give step by step instructions during printer setup. IMHO, the number of checkboxes and choices would not increase, but reduce over time :-)

Is creating a one-click printer setup a solvable problem?

What I can do is to look up a few commits which add a choice or a checkbox and tag them. Configtool code is pretty repeating, so procedures can be repeated at another location for another #define.

Other than that I agree, making inclusion into Configtool mandatory is asking a lot. Probably too much. Having a Github issue here makes sure such things don't get forgotten.

phord avatar Aug 18 '16 15:08 phord

I agree, ConfigTool should be more GUI and less checkboxes. But we can still have a happy medium between the GUI and the source code. The help text is a good example, but it's also a bad example.

Help text coming from the code is good because it means we only have to edit it in one place and it propagates to the GUI automatically.

Help text coming from the code is bad because it reads like source code comments, including instructions about the various options such as "uncomment the one you use and comment the others" and "#ifndef required for Arduino compatibility." Such instructions have no meaning in the GUI and do not serve to describe the option for the user in any meaningful way.

screenshot from 2016-08-18 11-41-39

It is essential for the GUI to know there is a DISPLAY_TYPE option because we want to put that in a relevant place in the GUI and not mashed in with all the PIN options, for example. We want to ensure the GUI and the code are always in sync with respect to the specific DISPLAY_TYPEs supported. We can attempt to synchronize this manually, but it is extra work for us and a possible source of errors. In engineering terms we say it is WET, and it is not DRY.

Of course we also want the GUI to be in sync in other regards, such as the fact that there is a DISPLAY_TYPE at all. Maybe we can create a "Miscellaneous" page which can hold all the options which do not get assigned a specific location in the GUI already. For example, the perennially orphaned TX_ENABLE_PIN and RX_ENABLE_PIN would end on this page.

It seems we have already abandoned DRY code with the myriad default config files. But maybe we can work towards some sanity there if only by ensuring we "normalize" them all from the generic.*.h files whenever a change is made to one of those.

phord avatar Aug 18 '16 15:08 phord