Refactor of the Fsm to be more modular (component based) + various improvments

[Pierre-33]

Hey!
So this is the result of my last weeks of work. It's kind of a huge refactor, so let me know if you accept that as a PR or if you'd rather have me stay in my fork. I'll try to describe here the most noticeable change.

SenseoWifi.cpp has been slim down and now mostly handle setup and Homie things like Mqtt handlers. Among the most noticeable things :

• I noticed that you read the configuration variable before Homie' setup which results in invalid value
• HomeAssistant sensors are now automatically created thanks to the publication of HomeAssistant AutoDiscovery Config
• All the Fsm logic is now on separate files (one per states). And states are easily overidable. I thought I would need to override some states behavior with my button hack, but I turns out I didn't. However it should make things easier if some future Senseo machine behave differently.
• You'll find code here to handle my CustomizableButtonAddon, which is totally optional.
• Mqtt handlers now go through a Command pattern to communicate with the Fsm. So does the button when the Addon is installed.

The folder SenseoFsm contain all the states logic which is basically the same as before.
You'll find in that folder a component folder. Each of them represents a different functionality of the Senseo, like the Buzzer or the CupDetector. You should recognize your code inside most of them. The nice thing is that most of them are totally optional. If you don't have a Buzzer, just don't add the BuzzerComponent when you create the Fsm :)

Regarding the LedObserver :

• I fix an issue when the boot time was longer than 3 * pulseContThreshold, the led could stay stuck in LED_unknown (it might have been introduced by slightly longer boot time with all my modifications, not sure about that)
• The LedObserver should now be thread safe, in your implementation it could happen every now and then that the interrupt kick in right in the middle of SenseoLed::updateState execution. It could result in inconsistency between ledChangeMillis and prevLedChangeMillis, or worst, totally random value into your float. (Note that I didn't encounter that issue myself, I just infer it from reading the code)
• My first thread safe implementation was using the Hardware Timer, unfortunately I learned the hard way that the tone() function uses it too... The code is still present in HwTimerLedObserver and as a result it's now quite easy to try different implementations of the LedObserver through a common interface.

I change all the formatting to something more VScode compliant. At first I tried to comply with the previous formatting but then I realize that it was most likely because your project started with the Arduino Ide and that it didn't make much sense with a modern IDE. Hope it wasn't a mistake...

In terms of pure features, here is what you can expect (line in bold require the CustomizableButtonAddon) :

• Home Assistant auto discovery
• Implementation of EzBuzzer with some nice tunes (I could use some help with the "sound design")
• Full brewing cycle (on -> heating -> ready -> brewing -> off) is now handled by the Senseo. No need for an HA automation anymore.
• Pressing one cup or two cups buttons on the Senseo trigger a full cycle
• If the cup is removed during brewing, the brewing is canceled
• If brewing is requested through Mqtt without a cup, the brewing cycle starts until the Senseo is ready and then wait for the cup before brewing
• Same things if the brewing is requested from the Senseo Buttons
• Decicated led blinking pattern for Unknown State and Brewing State
• Holding all the three buttons altogether will beep 3 times and then reset the Senseo
• Holding one cup or two cup buttons will raise a boolean in Mqtt (a switch on HA) that you can then use in an automation to program your coffee on the next morning. While this switch is raised, the led will blink once per minute
• Holding the power button will reset that switch

All that is quite fresh and would probably require a few weeks of testing to be sure everyting is smooth

[Pierre-33]

Sorry, I didn't meant to update this PR with my huge refacto, I wanted to create a new one instead. Since I don't now how to revert that, I'll update the description of this one.

[Pierre-33]

@ThomDietrich sorry to poke you here, did you get a chance to take a look at my PR?
Depending on your feedback, I'll either go my own way and probably print a custom PCB that add the R2R ladder to your design or contribute here and keep it as an optional add on to your design.

[ThomDietrich]

Hey! This is huge. Great effort on your side :)
The most obvious change is your structural change to the FSM. I was quite happy with it, as it was slim but functional. Yours certainly adds a significant step in complexity. I would need more than 10 minutes to digest it.

Of course I am interested in your improvements and fixes (EzBuzzer, the bugs you found, etc) but I certainly would want to understand everything. In the next couple of weeks I am going to hack a newer Senseo and will likewise have to change a few parts of the overall logic. While at it, I will take your fork for a testrun :)

Even though cool and lucrative for many, I will not have the time nor the urgent need to look into your hacked buttons.

Long story short: You should continue to maintain your branch! I will have a look at this PR in a couple of weeks, let's stay in touch :)

[Pierre-33]

Great, let me know if you need some help to wrap your head around it. But you'll see, it's not that more complicated, it certainly add a lot of files, but I don't think it add a lot of code and it might even make your life easier for your new Senseo. Or maybe I'm not objective because I'm used to that kind of component paterns :)

[ThomDietrich]

And I thought my degree of objects and two cascaded fsm would already make someone "used to component patterns" happy :D Jup, will let you know as soon as I am at it ;)

[bjornmorsman]

@Pierre-33

when i upload your fork and i enable home assistant discovery, the esp reboots. After that i get config.json error? If i leave autodicovery disabled the esp works just fine. Im a doing something wrong?

thanx in advance

[Pierre-33]

Hi, mine is working like a charm for a few months now, and I just check that forgot any commit on my computer, so I don't see why it's not working for you :-/

Any log or message that could be useful before the reboot?

Ha Autodiscovery require AsyncMQTTClient 0.9.0 to work, but if you didn't change anything, you should have it. You should look for "marvinroger/AsyncMqttClient @ ^0.9.0" in your platformio.ini file just in case it's missing.

The only difference between autodiscovery enable or disable should be the call to publishHomeAssistandDiscoveryConfig(). You can try to comment things out to pinpoint the faulty line.

I'm just realizing you must be using Thom PCB without the customizable button since I didn't share mine :-/ And I didn't test my fork without the add-on for quite a while now. I'll try to re read the code with that in mind to see if I can catch anything obvious.

Also, are you using the cup detector and the buzzer?

[bjornmorsman]

Hello Pierre,

i uploaded your fork to test again, when i configer everything the esp reboots en connects to the wifi. When i watch my broker the senseo topic gets updated but not for all sensors. The status of the senseo $state keeps at INIT, it never get to ready.

can you help me with this?

[Pierre-33]

Sorry, without any logs or any more informations, I have no clue on what could be going on your side :-/
Did you try to let the esp connected to your computer when it boot to see the logs?

[bjornmorsman]

Hello Pierre,

this is my log when connected to the pc, i also added the overview of the mqtt broker. so as you can see it looks like mqtt is working because some topics get populated but still the log says Sending initial information and never gets past that message.
Hoop you can help.

Reading 1: A0 = 12
Reading 2: A0 = 16
Reading 3: A0 = 16
Average Reading: A0 = 14
No Addon detected
💡 Firmware senseo-wifi (2.0)
🔌 Booting into normal mode 🔌
{} Stored configuration
  • Hardware device ID: 10521c01fe22
  • Device ID: senseo-keuken-morsman
  • Name: Senseo Keuken Morsman
  • Device Stats Interval: 60 sec
  • Wi-Fi:
    ◦ SSID: IoT
    ◦ Password not shown
  • MQTT:
    ◦ Host: 192.168.1.151
    ◦ Port: 1883
    ◦ Base topic: homie/
    ◦ Auth? yes
    ◦ Username: Bjorn
    ◦ Password not shown
  • OTA:
    ◦ Enabled? yes
  • Custom settings:
    ◦ cupdetector: 1 (set)
    ◦ buzzer: 1 (set)
    ◦ homeassistantautodiscovery: 1 (set)
↕ Attempting to connect to Wi-Fi...
✔ Wi-Fi connected, IP: 192.168.11.129
Triggering WIFI_CONNECTED event...
↕ Attempting to connect to MQTT...
Sending initial information...

[Pierre-33]

Are you confortable enough with C++ to add a few logs?

If so, could you add a bunch of "Serial.println("some unique string to identify this log");" in SenseoWifi.cpp:

• At the very beginning of loopHandler()
• At the very end of setup(), one before Homie.setup() and the other one after
• still in setup(), one before and after senseoNode.advertise("pendingCommands") and one after senseoNode.advertise("processedCommands")
• One for each case of the switch() in onHomieEvent()
• At the beginning of setupHandler(), before and after mySenseo.setup()

Which version of the PCB are using? Which "option" did you choose. Did you have a buzzer? A cup detector? A reset Button wired to D4? Did you used my version of the PCB with the customizable button add-on?

Are you sync to latest on my forks? (and NOT on the old branch within it).

[bjornmorsman]

Hello Pierre,

thanx for your message, iam not very confortable with C++ so i shall see of i can add some logging. today i was looking around in your programming and i found some strange behavour. mqtt topics get populated until PendingCommands so i though let i comment out that rule.

after a flash i got some sort of bootloop so i checked the programming again. after i comment out the next 3 lines the senseo boots and functions.

    senseoNode.advertise("pendingCommands").setName("Current Commands").setDatatype("string").setRetained(false);
    senseoNode.advertise("processedCommands").setName("Current Commands").setDatatype("string").setRetained(false);
    senseoNode.advertise("programContext").setName("Program Context").setDatatype("string");

so i think there is the problem, PendingCommands and processedCommands are nowhere declaired so is it still in use?
the programContext rule is declaired so i think that is still used for something, if is leave that rule in place my esp ends up in a boot loop.

i shall check this weekend if i can add some Serial.printIn to the programming so we can see what kind of errors i got.
i have pcb version 1.7 buzzer and cup detector and discovery enabled no reset button. stil without you addon board because im waiting for the pcd arives

Thanx in advance and have a nice weekend.

[Pierre-33]

i was looking around in your programming and i found some strange behavour. mqtt topics get populated until PendingCommands so i though let i comment out that rule.

Yes, I noticed that as well, that's why I asked some extra logs around those lines :)
They are used in CommandComponent::updateSenseoNode().

We'll see what the logs will tell us, but my bet would be that the program is stuck either inside advertise("pendingCommands") or advertise("processedCommands").
If that's the case, we'll probably need to look into the version of the libs you use, and maybe into your MQTT broker. Is it up to date? did you try to reboot it? Did you take a look at the log on the broker?

On a totally different idea, can you try to comment all those lines in setup() ?

if (!useCustomizableButtonsAddon)
{
    // setResetTrigger need to be call before the setup and therefor can't be based on the configuration variable
    pinMode(resetButtonPin, INPUT_PULLUP);
    Homie.setResetTrigger(resetButtonPin, LOW, 5000);
}

When I don't detect the CustomizableButtonsAddon (like in your case), I configure D4 to be used as the resetButtonPin, since you don't have one it might introduce weird behavior. I honestly don't bet too much on that since we would most likely see the reboot in the logs, but it could worse a try.

Another interesting test to do would be to try @ThomDietrich repo instead of my branch to see if the issue is only present on my fork or if it might be something with your PCB.

Have a nice weekend too!

[bjornmorsman]

Hi Pierre

i did this but no logs are shown, can you tell me if i did it correct?

    senseoNode.advertise("debug").setName("Debugging Information").setDatatype("string").setRetained(false);    
    senseoNode.advertise("opState").setName("Operational State").setDatatype("enum").setFormat("SENSEO_unknown,SENSEO_OFF,SENSEO_HEATING,SENSEO_READY,SENSEO_BREWING,SENSEO_NOWATER");
    senseoNode.advertise("ledState").setName("Led State").setDatatype("enum").setFormat("LED_unknown,LED_OFF,LED_SLOW,LED_FAST,LED_ON");
    senseoNode.advertise("power").setName("Power").setDatatype("boolean").settable(powerHandler);
    senseoNode.advertise("brew").setName("Brew").settable(brewHandler).setDatatype("enum").setFormat("1cup,2cup");
    senseoNode.advertise("brewedSize").setName("Brew Size").setDatatype("string").setRetained(false);
    senseoNode.advertise("outOfWater").setName("Out of Water").setDatatype("boolean");
    senseoNode.advertise("cupAvailable").setName("Cup Available");
    senseoNode.advertise("cupFull").setName("Cup Full");
    Serial.println("before pendingCommands");
    senseoNode.advertise("pendingCommands").setName("Current Commands").setDatatype("string").setRetained(false);
    Serial.println("after pendingCommands and before processedCommands");
    senseoNode.advertise("processedCommands").setName("Current Commands").setDatatype("string").setRetained(false);
    Serial.println("after processedCommands");
    senseoNode.advertise("program").setName("Program").settable(programHandler).setDatatype("enum").setFormat("1,2,clear");
    senseoNode.advertise("hasProgram").setName("Has Program").setDatatype("boolean");
//    senseoNode.advertise("programContext").setName("Program Context").setDatatype("string");
    senseoNode.advertise("buttonsAddon").setName("Use Customizable Buttons Addon").setDatatype("boolean");

[Pierre-33]

Yes, that look right, if it pass the compilation it should work. Did you put any other Serial.println, like at the beginning of the function or did you start just by those?
It seems you change the order of the advertise right? Maybe it's not about a specific advertise, but about a number of advertise. If you delete all the topic with MQTT explorer and try again, which topic did you see?

[Pierre-33]

@ThomDietrich , sorry for all the noise here, maybe it's better if we move the conversation into an issue directly on my forks.
What do you think @bjornmorsman ?

[bjornmorsman]

hello Pierre,

let move it to your fork, can you create an issue? the option is not available at your fork

[Pierre-33]

I think I just activated the option, can you try?