Add inverter base class and related interfaces#170
Conversation
|
@Awienert needed a longer journey ... ;-) - was a lot broken ... But I followed your approach, but restructered a little... So please finalize your specific integrations for victron src/interfaces/inverters/victron.py And if done and tested we can go to develop branch. |
|
@ohAnd Sorry for the broken parts — my environment was working 🙂 |
|
@ohAnd Du sag mal kann ich meinen Branches irgendwie in HA testen, möchte das mit Victron Hardware testen ob es funktioniert. Ein reiner Unittest reicht mir hier nicht aus. Ich dachte ich kannd as repository in HA hinzufügen aber das klappt irgendwie nicht. |
Hi Andreas, hab's mal auf den letzten Stand aus develop hochgezogen und zumindest für "meinen" Bestand erfolgreich getestet. Und die inverter tests versucht ein wenig aufzuräumen ... ;-) Bzgl. deiner Controls-Tests mit Victron bitte erstmal lokal testen - heisst: einfach in deiner Entwicklungsumgebung (z.B. VS Code) ausführen -> ...\EOS_connect> python.exe .\src\eos_connect.py - natürlich die Config entsprechend gefüllt haben. (siehe auch -> https://ohand.github.io/EOS_connect/developer/index.html#dev-setup) Als HA addon kann man es ebenso "lokal" also ohne image builds etc innerhalb HA testen -> dazu den Inhalt aus dem Addon z.B. aus develop in den Ordner /addon/eos_connect_test kopieren -> die neue src mit deinen Änderungen reinschieben - config.yaml vom addon in der version hochziehen - das lokale addon wie externe Addons, dann in HA installieren ... -> ist im Netz allgemein gut beschrieben Aber wenn es lokal bei dir läuft und den Victron mittels Override z.B. sauber ansteuert - dann können wir auch zeitnah in den develop gehen und damit die Basis Pipeline nutzen. |
|
@Awienert wie weit bist du ;-) ... würde den Umbau des inverter interfaces gerne mit Deiner Integration abschliessen und in den dev übernehmen, damit wir weitere inverter aufnehmen können |
|
@ohAnd also Avoid Discharge, allow Charge und Charge from Grid funktionieren soweit. Getestet mit einem 3 Phasen System. Ist nicht ganz einfach mit Victron. Unittests müssen noch überarbeitet werden. Ich werde noch ein paar Worte schreiben wie es mit Victron umgestzt ist. Ich habe den aktuellen WIP gepushed. Eine neue Inverterklasse zu implementiren ist ja unabhäng von der Victron Klasse möglich. Die Basis steht ja. |
Jep, aber der Umbau auf die generische Art und Weise ist Teil dieses PRs --- daher wäre es gut, wenn wir hier zum Ende kommen, so dass #207 hier aufsetzen kann - über rebase auf develop, sobald dieser hier auf dev ist ... (hint for -> @chriszero) sonst müsste erst der "noch unfertige" Victron wieder herausgebaut werden und in einem neuen PR dann final integriert werden... |
|
Sobald der PR gemerged ist, kümmere ich mich drum. |
feat: Port InverterHA to BaseInverter architecture (PR ohAnd#170) - Add src/interfaces/inverters/inverter_ha.py (BaseInverter subclass) - Register 'homeassistant' type in InverterFactory and __init__.py - Add tests/interfaces/inverters/test_inverter_ha.py with BaseInverterTestSuite compliance + 52 HA-specific tests (65 total) - Prepared for rebase onto develop after PR ohAnd#170 merge
|
@ohAnd mein letzter push umfasst folgende Funktionen. Avoid Discharge, allow, Discharge sowie force charge für ein Victron Multiplus 3 Phasensystem. So hat es bei mir funktioniert. Für develop ok. |
|
@Awienert nach rebase (merge) zum letzten develop wären wir klar zum merge ... kannst du dir eben noch die 2 comments anschauen ... dann können wir hier zumachen... ;-) |
|
@ohAnd eine Sache wäre noch wichtig. Ich denke es benötigt noch eine Methode welche aufgerufen wird wenn EOS-Connect beendet wird. Um die Inverter in ihren Standardmodus zurückzuschalten und die Verbindung sauber zu schließen. Was denkst du? Idealerweise wird die Funktion def disconnect_inverter(self) -> bool: aufgerufen. Was sagst Du? |
hatte ich so schon im Transfer zur Abstraction mit drin, da im "alten Fronius" schon enthalten -> siehe hier https://github.com/ohAnd/EOS_connect/pull/170/changes#diff-4c6942772ab76e86fb131b5cc142b3e9439d363596f2c4fa3bf8c708b2df6c87R849 und in der abstarction hier -> https://github.com/ohAnd/EOS_connect/pull/170/changes#diff-157ef3cbc6015cd26e21644e5cc8e92e6308354ba8eb56ff7646bc7f72fb317dR145 final beim shutdown hier -> https://github.com/ohAnd/EOS_connect/pull/170/changes#diff-ca484ef3f317da85eea2c652870b2bacb69d9015c88ee3d872f345616c16b787L2046 daher "einfach" im Victron interface mit Inhalt hinterlegen - was hier der Standard ist - oder ähnlich wie Fronius die "Vorher- Werte" zurückschreiben |
|
@ohAnd ich abe noch ein paar Änderungen vorgenommen und den Shutdown integriert. Läuft erstmal... |
…ify code ownership and review process
ohAnd
force-pushed
the
feature/inverterbaseclass
branch
from
7730207 to
601fc6f
Compare
- Fixed import paths in all inverter files (base_inverter → inverter_base) - Added supports_extended_monitoring attribute to BaseInverter class - Added api_set_max_pv_charge_rate method to NullInverter - Restored test files from PR ohAnd#170 branch - Tests: 134 passed (1 pre-existing failure unrelated to imports) - Ready for merge to develop branch This completes the recovery from the rebase operation, with all import structure issues resolved and tests validating the integrations.
- Fixed import paths in all inverter files (base_inverter → inverter_base) - Added supports_extended_monitoring attribute to BaseInverter class - Added api_set_max_pv_charge_rate method to NullInverter - Restored test files from PR ohAnd#170 branch - Tests: 134 passed (1 pre-existing failure unrelated to imports) - Ready for merge to develop branch This completes the recovery from the rebase operation, with all import structure issues resolved and tests validating the integrations.
- Added supports_extended_monitoring_default class attribute to BaseInverter - Set instance attribute in __init__ from class default - Added api_set_max_pv_charge_rate method to NullInverter - Fixed test_set_mode_force_charge_sets_percentage_and_current to use correct 500W value - All 135 tests now passing
- README.md: Developer guide for adding new inverter implementations - ccgx_registers.py: Legacy register definitions (kept for reference alongside ccgx_registers_all.py) - victron_old.py: Previous Victron implementation version (kept for archival)
ohAnd
force-pushed
the
feature/inverterbaseclass
branch
from
265c347 to
b275669
Compare
…king Problem: ModbusTcpClient import was inside try/except block, so when pymodbus is not available (e.g., in CI/CD without dependencies), the name doesn't exist at module scope for monkeypatching. Solution: Define ModbusTcpClient = None at module level before the try/except block. This ensures the name always exists and can be monkeypatched by tests, regardless of whether pymodbus is installed. This fixes test failures in GitHub Actions CI where pymodbus is not pre-installed with the Python environment. - Tests pass locally: ✅ 135/135 - Tests now work in CI: ✅ All Victron tests can be mocked
- Removed Fronius V2 and Victron inverter interface implementations. - Updated inverter factory to check for required configuration keys for Fronius inverters. - Modified test configurations for Fronius legacy and V2 tests to include user and password. - Deleted outdated test files for inverter factory and Fronius V2.
…r classes - Create dedicated EvccInverter class for EVCC external control mode - Refine NullInverter to pure display-only implementation - Upgrade type detection from class name strings to isinstance() pattern for better type safety - Improve factory logic and export EVCC inverter from package - Fix error logging to accurately reflect initialization failures vs normal modes - Internationalize code: translate German comments to English (src/eos_connect.py, src/interfaces/inverters/victron.py) - Update factory test to verify correct EVCC inverter type creation
|
@Awienert sorry für das Durcheinander da oben ... bin beim rebase zu dev falsch abgebogen ... ;-) Zur letzten Sicherheit bitte victron nochmal real testen (ob dahingehend noch alles "am richtigen Platz" ist) - dann kommt der finale merge auf dev |
…restore logic on real Victron system
|
@ohAnd kein problem... du machts das super ! Also alles getestet und funktioniert soweit super. Anbei eine Beschreibung was in victron.py nun passiert. Logik der ESS-Steuerung durch EOS Connect Im Modus „Discharge Allowed“ verbleibt das Victron ESS-System im normalen Betriebszustand. Voraussetzung dafür ist, dass das ESS aktiviert ist und sich mindestens in einem der Modi „Optimized with BatteryLife“ oder „Optimized without BatteryLife“ befindet. In diesem Zustand arbeitet das ESS weiterhin nach der internen Victron-Logik. Wenn von EOS Connect der Modus „Avoid Discharge“ aktiviert wird, schaltet das System das ESS in den Modus External Control. Anschließend wird über die VE.Bus-Register der Leistungs-Sollwert des MultiPlus auf allen drei Phasen auf 0 W gesetzt. Dadurch wird verhindert, dass Energie aus der Batterie entladen oder aus dem Netz bezogen wird. Das System verhält sich somit neutral gegenüber dem Netz. Voraussetzung hierfür ist ein dreiphasiges Victron MultiPlus-System. Die Unterstützung für einphasige Systeme muss noch geprüft werden. Wird von EOS Connect der Modus „Charge from Grid“ aktiviert, wird ein positiver Netzleistungs-Sollwert an den MultiPlus übergeben. Dieser Sollwert entspricht der gewünschten Ladeleistung der Batterie aus dem Netz. Der MultiPlus regelt daraufhin den Netzbezug entsprechend und nutzt die bezogene Energie zum Laden der Batterien. Wenn EOS Connect beendet oder geschlossen wird, wird der ursprünglich aktive ESS-Modus automatisch wiederhergestellt. Dadurch wird sichergestellt, dass das Victron-System wieder vollständig in den normalen ESS-Betrieb zurückkehrt. VG |
feat: Port InverterHA to BaseInverter architecture (PR ohAnd#170) - Add src/interfaces/inverters/inverter_ha.py (BaseInverter subclass) - Register 'homeassistant' type in InverterFactory and __init__.py - Add tests/interfaces/inverters/test_inverter_ha.py with BaseInverterTestSuite compliance + 52 HA-specific tests (65 total) - Prepared for rebase onto develop after PR ohAnd#170 merge
feat: Port InverterHA to BaseInverter architecture (PR ohAnd#170) - Add src/interfaces/inverters/inverter_ha.py (BaseInverter subclass) - Register 'homeassistant' type in InverterFactory and __init__.py - Add tests/interfaces/inverters/test_inverter_ha.py with BaseInverterTestSuite compliance + 52 HA-specific tests (65 total) - Prepared for rebase onto develop after PR ohAnd#170 merge
- Fixed import paths in all inverter files (base_inverter → inverter_base) - Added supports_extended_monitoring attribute to BaseInverter class - Added api_set_max_pv_charge_rate method to NullInverter - Restored test files from PR #170 branch - Tests: 134 passed (1 pre-existing failure unrelated to imports) - Ready for merge to develop branch This completes the recovery from the rebase operation, with all import structure issues resolved and tests validating the integrations.
Add inverter base class abstraction and factory pattern for extensibility Integrate Victron inverter support (contributed by @Awienert) as the catalyst for comprehensive inverter architecture refactoring. ## Key Changes ### Core Architecture - Introduce BaseInverter abstract base class defining unified interface for all inverter implementations, eliminating code duplication and enforcing consistency - Implement InverterFactory pattern for dynamic inverter instantiation based on configuration type, replacing hardcoded conditional logic ### Package Reorganization - Restructure inverters into dedicated inverters/ subpackage with clean separation of concerns and consistent module naming - Move all inverter implementations under inverters/ (Fronius legacy/V2, Victron, EVCC, NullInverter) ### Inverter Implementations - FroniusLegacy: Inherit from BaseInverter, maintain full API compatibility - FroniusV2: Inherit from BaseInverter with enhanced firmware-based auth - VictronInverter: New implementation for Victron systems (contributed by @Awienert) - EvccInverter: Placeholder for external EVCC control - NullInverter: Display-only mode when no inverter is configured ### Application Updates - Replace hardcoded inverter type checks with factory method calls - Update inverter status checks to use isinstance() for type checking - Simplify main application logic by delegating instantiation to factory ### Developer Experience - Add comprehensive README in inverters/ documenting how to add new inverter types - Establish CODEOWNERS file designating @Awienert as maintainer of Victron code - Document in CONTRIBUTING.md the code ownership model ## Benefits - **Extensibility**: Adding new inverter types now requires only implementing BaseInverter (no changes to main application logic) - **Consistency**: Unified interface ensures all inverters support the same operations and behave predictably - **Maintainability**: Clear separation of concerns and inheritance hierarchy - **Future-proof**: Supports arbitrary inverter backends without architectural changes ## Backward Compatibility - All existing configuration formats remain supported - Deprecated interface names (fronius_gen24_v2) continue to work - No breaking changes to external APIs
feat: Port InverterHA to BaseInverter architecture (PR #170) - Add src/interfaces/inverters/inverter_ha.py (BaseInverter subclass) - Register 'homeassistant' type in InverterFactory and __init__.py - Add tests/interfaces/inverters/test_inverter_ha.py with BaseInverterTestSuite compliance + 52 HA-specific tests (65 total) - Prepared for rebase onto develop after PR #170 merge
3 participants
Draft – Proposal for New Inverter Architecture
This PR is a draft and a proposal introducing a new inverter architecture.
What's new
BaseInverter: shared abstract base class providing a unified interface and reducing duplicate logic.
InverterFactory: creates inverter instances based on config, making it easier to add new inverter types.
Why
cleaner structure
easier extensibility
consistent API for all inverters