fix(solis): queue entity events for the component loop - #4875
Open
mgazza wants to merge 1 commit into
Open
Conversation
Solis entity callbacks perform real API reads and writes inline. They are invoked from the HA component's loop (ha.py -> trigger_callback), not Solis's own, because standalone Predbat runs every component in its own thread with its own asyncio.run() loop (hass.py). The ClientSession belongs to the Solis loop, so issuing a request from the callback loop raises inside aiohttp; the handler swallows it and returns normally, and the user is told a write succeeded that never reached the inverter. Ohme and Octopus already avoid this: their callbacks only append to a queue and the work happens in their own run(). Solis now does the same. select_event, number_event and switch_event become thin stubs that queue, the existing bodies become *_event_handler, and run() drains the queue on the Solis loop before polling. A failing handler is logged and the rest of the queue still drains, since the queue is in memory only and anything dropped is lost outright. This keeps one session on one loop, so connection reuse, socket lifetime and teardown are all unchanged — no per-loop session juggling, and nothing special for injected test sessions. Existing event tests now call the handlers directly, which is what they were always exercising. Two new tests cover the dispatch itself: that a callback queues rather than executing on the calling loop, and that one failing event does not strand the rest. MockSolisAPI gains queued_events, as it hand-rolls the state the real __init__ would set. Verified with the repository harness (unit_test.py --test solis, which --quick skips): the suite passes, and the two pre-existing unclosed-session warnings and the multi_car_iog failure are present on a clean tree too. Mutation-checked — making select_event call its handler inline again fails the new test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Solis entity callbacks perform real API reads and writes inline —
solis.py:2650,:2880,:3056.But those callbacks run on the HA component's event loop, not Solis's own. Standalone Predbat runs every component in its own thread with its own
asyncio.run()loop (hass.py:217,:223), and the HA interface is one of those components — its websocket loop awaitstrigger_callback()directly (ha.py:619).The
ClientSessionis bound to the Solis loop. Issuing a request from the callback loop raises inside aiohttp, the handler catches it and returns normally, and the user is told a write succeeded that never reached the inverter.Why Solis specifically
Ohme and Octopus already avoid this — their callbacks only append to a queue and do the work in their own
run():octopus.py:591), executed inrun()(:641)ohme.py:689), drained inrun()(:239)Change
Solis follows the same pattern.
select_event,number_eventandswitch_eventbecome thin stubs that queue; the existing bodies become*_event_handler;run()drains the queue on the Solis loop before polling. A failing handler is logged and the rest of the queue still drains — the queue is in memory only, so anything dropped is lost outright.This deliberately does not touch session lifetime. One session stays on one loop, so connection reuse, socket lifetime and teardown are all unchanged, and injected test sessions keep working.
Tests
Existing event tests now call the handlers directly — that is what they were always exercising.
Two new tests cover the dispatch itself: that a callback queues rather than executing on the calling loop, and that one failing event does not strand the rest of the queue.
MockSolisAPIgainsqueued_events, since it hand-rolls the state the real__init__sets.Verified with the repository harness —
unit_test.py --test solis, which--quickskips:Unclosed client sessionwarnings and themulti_car_iogfailure are present on a clean tree too, so neither is introduced here.select_eventcall its handler inline again fails the new test withcallback executed API work on the calling loop.Note
This supersedes #4874, which tried to solve it at the session layer by rebinding the
ClientSessionper loop. That was the wrong level — it broke injected test sessions, leaked a session per rebind, and destroyed connection reuse in exactly the alternating-loop case it targeted. Closing that in favour of this.