Break up, refactor, and enforce style on the E-TKT firmware code

[sabeechen]

I think it would be a good idea to refactor the E-TKT from its current (largely) single file structure into one that encapsulates its functionality into multiple classes. There are many advantages to this, but the biggest I see are:

• Breaking functionality into smaller parts will make it easier to follow logic.
• Smaller files are easier to read, and therefore more maintainable.
• Breaking up functionality makes it easier to avoid conflicting changes if multiple people work on the project simultaneously.
• Bugs are less likely if the state for the device isn't all tangled together everywhere.
• Extending functionality is easier to do (ie for different hardware) when responsibilities are clearly defined by a class structure.
• Adding tests will be easier if most classes depend on abstractions of the hardware rather than the arduino methods themselves.

These are the typical upsides of breaking up and encapsulating functionality. As usual it also comes with some downside, if things are broken up naively then it can often become more work to add functionality because the relationships between parts doesn't reflect what they actually need to do. My goal would be to avoid that, and I think I'm familiar enough with the project to do it well. I have a lot of professional experience doing this kind of thing, though not specifically with C++.

Here is how I'm initially thinking of breaking things up:

• Printer.h - A "main" class created at the start which initializes everything and keeps track of global state like print progress, busy, etc
• Helpers.h - Static utility methods, like the ones for handling utf-8 text.
• Carousel.h - Manages everything having to do with the carousel geometry, its stepper motor, and homing.
• Sound.h - Manages making sounds/music with the buzzer.
• Display.h - Manages all the logic for displaying info on the oled.
• Server.h - Sets up the webserver, its callbacks, and passing info from requests to where they need to go.
• WiFi.h - Handles the logic for connecting to WiFi, AP Config mode, mdns, etc.
• Button.h, Light.h, Motor.h - To handle smaller units of hardware. These classes will probably be very simple.

While actually untangling the relationships whats needed might change, but I think its likely things will look close to this.

From my personal experience its better to make big changes like this quickly all at once to minimize the time it impacts people with pending changes. Id want to break it all up quickly and coarsely in one PR, when go through and refactor/clean up individual classes by themselves at a more leisurely pace.

Alternative would be to check-in each new class one at a time, slowly carving out functionality from LabelMaker.cpp. This is less disruptive to concurrent contributors but can take a lot longer to complete.

As things are broken up I'd like to move toward enforcing a C++ style, the most obvious choice is probably Google C++ Style with clang, but any of the common alternatives would be fine so long as its adhered to, including something we make custom. Most projects choose one of the well-known styles (Google, Microsoft, Mozilla, etc) and then just override rules as they see fit.

@andreisperid You have your finger on the pulse of whats going on with this project currently. Do you think this is a good idea, and if so do you think now is a good time to do it?

[andreisperid]

I think that's a great initiative, @sabeechen

From a roadmap point of view, these are the major things that might happen from now on:

• improvements on hardware, that might affect the firmware somehow;
• bugfixes that might appear with new browser versions;
• integration with other systems such as the Home Assistant, Matter, etc, which will probably talk to the Server.h;
• forking into other similar projects (lets say a braille embossing machine? I'm thinking of it ;)

So, right now I understand the E-TKT's archetype is pretty solid, which means the approach you brought makes sense and will not prevent any development. Instead, it might even improve as collaboration will surely take advantage of that clear structure.

Also, for me it will be surely a great opportunity to learn, I'm looking forward to it! I trust whatever you bring ;)

[skrutt]

Great idea, i agree! I would probably go for one piece at a time, simpler to get an overview and there are a lot of moving pieces. Today the code is quite interleaved and many things rely on each other, for example the print progress is updated in a display function. Sort of makes sense since that functionality is available there but not if you are looking to create functional modules with clean, well defined interfaces. Will be interesting to see where it goes, i would probably go for a function based layout instead (display, print, music, web), but both have merit.

If you do a music module, would you make that non-blocking? Cuz that would be cool 😄

[sabeechen]

Yeah, its got some oddities. But maybe it won't for much longer? I made a proposal for my massive refactor in this PR. Non-blocking execution is on my mind, but won't be in this change. Its a tricky thing to get working in the device's state model but possible down the line.

[andreisperid]

Implemented on #46