Ideas for refactoring

[tmjo]

To discuss ideas for refactoring of the library

[tmjo]

Some thoughts from my side:

• organize tubdata in a struct, as well as "system" data such as wifi etc etc in a system struct.
• rename to *.cpp with corresponding *.h file(s) for correct CPP-style (not Arduino) to fit both Platform.io and future usage of library externally (ESP Home for instance).
• Get rid of Strings (heap-killers)
• Split HA functions (mostly for commands) so that they call a library function independent of HAMQTT (for instance let the library have a setLight() function which is then called in the HAMQTT logic. I think it is good practice and makes things easier if we one day want to make the library more generic.
• Move some stuff to functions to make loop() and setup() cleaner and easier to read
• General washing of names to make them more representable with what we now know and split HA-stuff from tub-stuff (struct helps here).

Just some brainstorming. I uploaded this branch for discussion only. It is compiling and most of it is tested, perhaps not all. Still lots of stuff there I know is not correct and have to verify, so only intended for the sake of discussion. Lots of inspiration from cribskip's code, and I believe we could also do some bitwise stuff to get actual status of pumps and stuff as confirmed in on of the other threads.

[netmindz]

Think I agree with much of what you have said there, some of what you have in your branch or listed above is in #42

In terms of transition to bitwise for the pump stuff, while that might be neater, think that might be less readable, not really much difference in performance and I would wonder if this specific refactor represents value for money in terms of dev effort

[tmjo]

It is very easy to do, like my (commented) example: bitRead(Q_in[7],0) for bit0 of Q_in[7].

If the pumps, heater, light and perhaps even modes (or at least like circulation pump on and priming info) is bitcoded, I think it makes it simpler since there are fewer combinations.

For instance, pump 1 is defined by bit 1 and 2 only. Pump 2 bit 3 and 4. It would mean is not necessary to check for byte with both pump1 and pump2 running, since we would know their state only by checking the two specific bits. It would remove number of ifs/switch case in the verification.

0		00000000	All off			ok
1		00000001	Pump1 Low		ok
2		00000010	Pump1 High		ok
4		00000100	?			Would make sense if Pump1 Off, Pump2 Low
6		00000110	Pump1 High Pump2 Low	ok
7		00000111	Pump1 Off Pump2 Low	Is this correct? If bitwise 11 would mean both low and high for pump1
8		00001000	Pump1 Off Pump2 High	ok
9		00001001	Pump1 Low, Pump2 High	ok
a		00001010	Pump1 High, Pump2 High	ok

But let's look at it step by step :)

[netmindz]

FYI saw some issues while testing the library branch, but when I switched back to main I was also seeing similar issues so don't think it's related to the refactor.

E.g pin5 going high before I had finished sending commands

[netmindz]

I've just created a PR for the merge of the split of the HomeAssistant code from the common balboaGL class - now it's own repo as was the only way to get ESPHome custom component to build properly

If you have any changes in your own branches, it might be best you tackle these before I merge this PR #42