A few markers, @thatthing
-
Just a little thing, but the interrupt pin is pulled low, not high, when the panel has an event available.
-
For talking to the I2C bus, the cleanest implementation is to follow the ESPHome-way of doing this. This means that you write a component that dervies from the
esphome::i2c::I2CDeviceclass (see the code on github]. It works with the I2C bus(es) as defined in the device yaml configuration file. This parent class provides some useful functions to work with the I2C bus, and it has a built-in option for handling communication timeouts. -
I was wondering if the PollingComponent was the correct way to handle the interrupt pin state detection. Turns out it is, and that setting up interrupt handlers is not the way to go with ESPHome. So that was a good learning point for me.
-
About comparing arrays: since we’re looking at C++ and not plain C, I would opt for using the
std::arrayclass (some docs on this one). You can use the==equality operator for twostd::arrayobjects to test if they match. Another way would of course be to use C-style arrays as you did, and write a little loop that compares the bytes one at a time yourself. -
For parsing the bytes as received from the front panel, I would probably not setup a mapping of all possible messages, but I would write a parser class that would implement more of a decision tree to find the message type. The structure would be a starter method that would look at the first byte, to call a method for processing the next byte or bail out with an error if the message cannot be parsed. Every followup function would do the same, until eventually a conclusion is reached about the resulting message code. There are other ways to implement such parser, but the important concept here would be the use of a decision tree instead of a lookup map.Using a tree takes the least steps to get from an input to a resulting message code. I made a drawing to show the paths for this:
As you can see, I suggested a computation based on the last two bytes for the slider’s touch and release events. There is a distinct pattern there, which could be used to translate the two bytes into the correct slider value. Forking the decision tree further would be feasible too of course. It might even be easier to detect the invalid cases that way.
Of course, in such tree-based parser, you don’t need four functions in succession to handle the first four bytes. The first function can check for the first four bytes to be 04, 04, 01, 00, and then call the next function or bail out based on the fifth byte. In pseudo code:
enum EventType {
UnknownMessage,
PowerTouch,
PowerRelease,
ColorTouch,
ColorRelease,
Level1Touch,
Level1Release,
// etc....
};
EventType parse(FrontPanelMessage message) {
if (message[0] != 0x04) return UnknownMessage;
if (message[1] != 0x04) return UnknownMessage;
if (message[2] != 0x01) return UnknownMessage;
if (message[3] != 0.00) return UnknownMessage;
if (message[4] == 0x01) return parse_power_button_event(message);
if (message[4] == 0x02) return parse_color_button_event(message);
if (message[4] == 0x03) return parse_slider_touch_event(message);
if (message[4] == 0x04) return parse_slider_release_event(message);
return UnknownMessage;
}
EventType parse_power_button_event(FrontPanelMessage message) {
if (message[5] == 0x01) return parse_power_button_touch_event(message);
if (message[5] == 0x02) return parse_power_button_release_event(message);
return UnknownMessage;
}
// etc....
As for storing the resulting event: for the slider events, it would be nice to have an actual event object, in which the slider level is stored as an integer value. That makes it easier to work with the resulting event (it prevents the need of a long switch statement to translate the event type to a slider level). Another way to handle this would be to provide a translation function from your code, that takes an event as input (e.g. Level12Touch) and would return its numerical value (12).
This was a bit of a brain dump, but I hope these pointers will help!
! Please keep up the good work!!
!
And 
