feat: Add error code sensor #18
Conversation
- Has no actual impact on anything, but good to know.
KazWolfe
force-pushed
the
feat/error-codes
branch
from
72f09c8
to
a0063bc
Compare
components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp
Outdated
Show resolved
Hide resolved
components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp
Outdated
Show resolved
Hide resolved
components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp
Outdated
Show resolved
Hide resolved
| @@ -133,6 +134,11 @@ void MitsubishiUART::update() { | |||
| hp_bridge.sendPacket(GetRequestPacket::getStatusInstance()); | |||
| hp_bridge.sendPacket(GetRequestPacket::getCurrentTempInstance()); | |||
| ) | |||
|
|
|||
| // TODO: get this every 60 seconds instead of every n loops | |||
| if (this->_updateLoopCounter % 10 == 0 && !ts_bridge) { | |||
There was a problem hiding this comment.
Don't check for !ts_bridge here, just send it anyway. Most of the other packets we send are already redundant with an MHK2 attached. If we want to try to cut down on bandwidth (is this a concern?), we should try to e.g. cache all received packets and only request new ones if we need them. But I think that's a separate change.
There was a problem hiding this comment.
My primary concern here revolves around eventually overloading the heat pump with Too Many Things At Once. 2400 baud is plenty for this use case, but it's still rather slow in general.
Assuming we keep requesting more and more information (especially so long as we transparently forward MHK packets), we might start making headaches for ourselves. c.f. specifically akamali/mhk1_mqtt noting the MHK1 has some rather strict timing.
See also #10 for a more broad scope, which might ultimately end with us maintaining our own state and not letting the thermostat talk to the heat pump directly (especially as Kumo does this).
I'm fine going either way here, but I also don't strictly see a need to have errors updated as frequently as active-state. I'm also rather more on the conservative side with how much we shove over the wire, so this may just be a preemptive automation.
There was a problem hiding this comment.
EDIT: I'll just be lazy for now and send this every time. See inlined comments and above, but I also don't want to deal with thinking of a better implementation for this yet. If we decide to go back to less-frequent polling, can just revert/remove commit b1f66f7 from this PR.
(Please don't resolve this conversation, this is something I'd want to stay easily visible.)
There was a problem hiding this comment.
I think the overloading concern is justified (I've already had some issues e.g. a20950d , and will leave open as requested), but I think the appropriate way to resolve is with a combination of minimum update periods, queue throttling, and caching. Would rather resolve this all at once for all the packets instead of piecemeal.
components/mitsubishi_uart/mitsubishi_uart-packetprocessing.cpp
Outdated
Show resolved
Hide resolved
While the loop counter overflowing shouldn't cause any particular problems, it's still probably best to just not let it overflow in the first place. 10,000 cycles should be enough for *any* scheduled event until there's a better system for infrequent events.
- Remove identified `auto` typing. - Simplify error packet else case to not care about logic flaws. - Check `raw_state` for publish checks, as that's what we're actually setting.
- Remove code to only check errors if the MHK isn't sending those packets. - Remove the every-n-ticks system. - Add TODO just in case this does cause issues later.
Pulled out of the PR #17 for maintainer sanity, and (mostly) resolves Sammy1Am/temp-issues#19.
ushorterror code.