From 5053731f31e6b5050cfe11644a4d64272d470635 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:49:59 +0000 Subject: [PATCH 01/31] rename and refactor dbus-opendtu --- dbus-opendtu.py | 157 ------------------------------------------- dbus_opendtu.py | 172 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 157 deletions(-) delete mode 100644 dbus-opendtu.py create mode 100644 dbus_opendtu.py diff --git a/dbus-opendtu.py b/dbus-opendtu.py deleted file mode 100644 index 8892262..0000000 --- a/dbus-opendtu.py +++ /dev/null @@ -1,157 +0,0 @@ -#!/usr/bin/env python -'''module to read data from dtu/template and show in VenusOS''' - - -# File specific rules -# pylint: disable=broad-except - -# system imports: -import logging -import logging.handlers -import os -import configparser -import sys - -# our imports: -import constants -import tests -from helpers import * - -# Victron imports: -from dbus_service import DbusService - -if sys.version_info.major == 2: - import gobject # pylint: disable=E0401 -else: - from gi.repository import GLib as gobject # pylint: disable=E0401 - - -def main(): - '''main loop''' - # configure logging - config = configparser.ConfigParser() - config.read(f"{(os.path.dirname(os.path.realpath(__file__)))}/config.ini") - logging_level = config["DEFAULT"]["Logging"].upper() - dtuvariant = config["DEFAULT"]["DTU"] - - logging.basicConfig( - format="%(levelname)s %(message)s", - level=logging_level, - ) - - try: - number_of_inverters = int(config["DEFAULT"]["NumberOfInvertersToQuery"]) - except (KeyError, ValueError) as ex: - logging.warning("NumberOfInvertersToQuery: %s", ex) - logging.warning("NumberOfInvertersToQuery not set, using default") - number_of_inverters = 0 - - try: - number_of_templates = int(config["DEFAULT"]["NumberOfTemplates"]) - except (KeyError, ValueError) as ex: - logging.warning("NumberOfTemplates: %s", ex) - logging.warning("NumberOfTemplates not set, using default") - number_of_templates = 0 - - - tests.run_tests() - - try: - logging.critical("Start") - - from dbus.mainloop.glib import DBusGMainLoop # pylint: disable=E0401,C0415 - - # Have a mainloop, so we can send/receive asynchronous calls to and from dbus - DBusGMainLoop(set_as_default=True) - - # region formatting - def _kwh(_p, value: float) -> str: - return f"{round(value, 2)}KWh" - - def _a(_p, value: float) -> str: - return f"{round(value, 1)}A" - - def _w(_p, value: float) -> str: - return f"{round(value, 1)}W" - - def _v(_p, value: float) -> str: - return f"{round(value, 1)}V" - # endregion - - paths = { - "/Ac/Energy/Forward": { - "initial": None, - "textformat": _kwh, - }, # energy produced by pv inverter - "/Ac/Power": {"initial": None, "textformat": _w}, - "/Ac/L1/Voltage": {"initial": None, "textformat": _v}, - "/Ac/L2/Voltage": {"initial": None, "textformat": _v}, - "/Ac/L3/Voltage": {"initial": None, "textformat": _v}, - "/Ac/L1/Current": {"initial": None, "textformat": _a}, - "/Ac/L2/Current": {"initial": None, "textformat": _a}, - "/Ac/L3/Current": {"initial": None, "textformat": _a}, - "/Ac/L1/Power": {"initial": None, "textformat": _w}, - "/Ac/L2/Power": {"initial": None, "textformat": _w}, - "/Ac/L3/Power": {"initial": None, "textformat": _w}, - "/Ac/L1/Energy/Forward": {"initial": None, "textformat": _kwh}, - "/Ac/L2/Energy/Forward": {"initial": None, "textformat": _kwh}, - "/Ac/L3/Energy/Forward": {"initial": None, "textformat": _kwh}, - "/Ac/Out/L1/I": {"initial": None, "textformat": _a}, - "/Ac/Out/L1/V": {"initial": None, "textformat": _v}, - "/Ac/Out/L1/P": {"initial": None, "textformat": _w}, - "/Dc/0/Voltage": {"initial": None, "textformat": _v}, - } - - if dtuvariant != constants.DTUVARIANT_TEMPLATE: - logging.critical("Registering dtu devices") - servicename = get_config_value(config, "Servicename", "INVERTER", 0, "com.victronenergy.pvinverter") - service = DbusService( - servicename=servicename, - paths=paths, - actual_inverter=0, - ) - - if number_of_inverters == 0: - number_of_inverters = service.get_number_of_inverters() - - if number_of_inverters > 1: - # start our main-service if there are more than 1 inverter - for actual_inverter in range(number_of_inverters - 1): - servicename = get_config_value( - config, - "Servicename", - "INVERTER", - actual_inverter + 1, - "com.victronenergy.pvinverter" - ) - DbusService( - servicename=servicename, - paths=paths, - actual_inverter=actual_inverter + 1, - ) - - for actual_template in range(number_of_templates): - logging.critical("Registering Templates") - servicename = get_config_value( - config, - "Servicename", - "TEMPLATE", - actual_template, - "com.victronenergy.pvinverter" - ) - service = DbusService( - servicename=servicename, - paths=paths, - actual_inverter=actual_template, - istemplate=True, - ) - - logging.info("Connected to dbus, and switching over to gobject.MainLoop() (= event based)") - mainloop = gobject.MainLoop() - mainloop.run() - except Exception as error: - logging.critical("Error at %s", "main", exc_info=error) - - -if __name__ == "__main__": - main() diff --git a/dbus_opendtu.py b/dbus_opendtu.py new file mode 100644 index 0000000..7ce244e --- /dev/null +++ b/dbus_opendtu.py @@ -0,0 +1,172 @@ +#!/usr/bin/env python +'''module to read data from dtu/template and show in VenusOS''' + +# File specific rules +# pylint: disable=broad-except + +# system imports: +import logging +import logging.handlers +import os +import configparser +import sys + +# our imports: +import constants +import tests +from helpers import * + +# Victron imports: +from dbus_service import DbusService + +if sys.version_info.major == 2: + import gobject # pylint: disable=E0401 +else: + from gi.repository import GLib as gobject # pylint: disable=E0401 + +config = None +paths = None +number_of_inverters = 0 +number_of_templates = 0 + +def initialize(): + """ Initialize the module """ + # pylint: disable=w0603 + global config, paths, number_of_inverters, number_of_templates + + # configure logging + config = configparser.ConfigParser() + config.read(f"{(os.path.dirname(os.path.realpath(__file__)))}/config.ini") + logging_level = config["DEFAULT"]["Logging"].upper() + + logging.basicConfig( + format="%(levelname)s %(message)s", + level=logging_level, + ) + + try: + number_of_inverters = int(config["DEFAULT"]["NumberOfInvertersToQuery"]) + except (KeyError, ValueError) as ex: + logging.warning("NumberOfInvertersToQuery: %s", ex) + logging.warning("NumberOfInvertersToQuery not set, using default") + number_of_inverters = 0 + + try: + number_of_templates = int(config["DEFAULT"]["NumberOfTemplates"]) + except (KeyError, ValueError) as ex: + logging.warning("NumberOfTemplates: %s", ex) + logging.warning("NumberOfTemplates not set, using default") + number_of_templates = 0 + + # region formatting + def _kwh(_p, value: float) -> str: + return f"{round(value, 2)}KWh" + + def _a(_p, value: float) -> str: + return f"{round(value, 1)}A" + + def _w(_p, value: float) -> str: + return f"{round(value, 1)}W" + + def _v(_p, value: float) -> str: + return f"{round(value, 1)}V" + # endregion + + paths = { + "/Ac/Energy/Forward": { + "initial": None, + "textformat": _kwh, + }, # energy produced by pv inverter + "/Ac/Power": {"initial": None, "textformat": _w}, + "/Ac/L1/Voltage": {"initial": None, "textformat": _v}, + "/Ac/L2/Voltage": {"initial": None, "textformat": _v}, + "/Ac/L3/Voltage": {"initial": None, "textformat": _v}, + "/Ac/L1/Current": {"initial": None, "textformat": _a}, + "/Ac/L2/Current": {"initial": None, "textformat": _a}, + "/Ac/L3/Current": {"initial": None, "textformat": _a}, + "/Ac/L1/Power": {"initial": None, "textformat": _w}, + "/Ac/L2/Power": {"initial": None, "textformat": _w}, + "/Ac/L3/Power": {"initial": None, "textformat": _w}, + "/Ac/L1/Energy/Forward": {"initial": None, "textformat": _kwh}, + "/Ac/L2/Energy/Forward": {"initial": None, "textformat": _kwh}, + "/Ac/L3/Energy/Forward": {"initial": None, "textformat": _kwh}, + "/Ac/Out/L1/I": {"initial": None, "textformat": _a}, + "/Ac/Out/L1/V": {"initial": None, "textformat": _v}, + "/Ac/Out/L1/P": {"initial": None, "textformat": _w}, + "/Dc/0/Voltage": {"initial": None, "textformat": _v}, + } + + +def register_service(): + """ Register the service """ + global number_of_inverters + + dtuvariant = config["DEFAULT"]["DTU"] + if dtuvariant != constants.DTUVARIANT_TEMPLATE: + logging.critical("Registering dtu devices") + servicename = get_config_value(config, "Servicename", "INVERTER", 0, "com.victronenergy.pvinverter") + service = DbusService( + servicename=servicename, + paths=paths, + actual_inverter=0, + ) + + if number_of_inverters == 0: + # pylint: disable=W0621 + number_of_inverters = service.get_number_of_inverters() + + if number_of_inverters > 1: + # start our main-service if there are more than 1 inverter + for actual_inverter in range(number_of_inverters - 1): + servicename = get_config_value( + config, + "Servicename", + "INVERTER", + actual_inverter + 1, + "com.victronenergy.pvinverter" + ) + DbusService( + servicename=servicename, + paths=paths, + actual_inverter=actual_inverter + 1, + ) + + for actual_template in range(number_of_templates): + logging.critical("Registering Templates") + servicename = get_config_value( + config, + "Servicename", + "TEMPLATE", + actual_template, + "com.victronenergy.pvinverter" + ) + service = DbusService( + servicename=servicename, + paths=paths, + actual_inverter=actual_template, + istemplate=True, + ) + +def main(): + """ Main function """ + initialize() + tests.run_tests() + + try: + logging.critical("Start") + + from dbus.mainloop.glib import DBusGMainLoop # pylint: disable=E0401,C0415 + + # Have a mainloop, so we can send/receive asynchronous calls to and from dbus + DBusGMainLoop(set_as_default=True) + + register_service() + + logging.info("Connected to dbus, and switching over to gobject.MainLoop() (= event based)") + mainloop = gobject.MainLoop() + mainloop.run() + except Exception as error: + logging.critical("Error at %s", "main", exc_info=error) + +if __name__ == "__main__": + main() From 3f24b5e732733324e53d7ccb2cfa0cb125e69939 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:50:15 +0000 Subject: [PATCH 02/31] add basic test wip --- test_dbus-opendtu.py | 51 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 test_dbus-opendtu.py diff --git a/test_dbus-opendtu.py b/test_dbus-opendtu.py new file mode 100644 index 0000000..601340b --- /dev/null +++ b/test_dbus-opendtu.py @@ -0,0 +1,51 @@ +import unittest +from unittest.mock import patch, MagicMock +import sys +import os + +# Mocking the dbus and other dependencies before importing the module to test +sys.modules['dbus'] = MagicMock() +sys.modules['vedbus'] = MagicMock() +sys.modules['gi.repository'] = MagicMock() +sys.modules['dbus.mainloop.glib'] = MagicMock() + +# Add the directory containing dbus_opendtu.py to the Python path +sys.path.insert(0, os.path.abspath(os.path.dirname(__file__))) + +from dbus_opendtu import register_service + +class TestRegisterService(unittest.TestCase): + @patch('dbus_opendtu.config', {'DEFAULT': {'DTU': 'opendtu'}}) + @patch('dbus_opendtu.DbusService') + @patch('dbus_opendtu.get_config_value') + @patch('dbus_opendtu.constants') + + def test_register_service(self, mock_constants, mock_get_config_value, mock_dbus_service): + def get_config_value_side_effect(key, *args, **kwargs): + if isinstance(key, dict): + key = key.get('key', 'mock_value') + return { + 'number_of_inverters': 2, + 'number_of_templates': 1, + 'DTU': 'openDTU' + }.get(key, 'mock_value') + + mock_get_config_value.side_effect = get_config_value_side_effect + #mock_dbus_service.return_value = MagicMock() + mock_dbus_service_instance = mock_dbus_service.return_value + mock_dbus_service_instance.get_number_of_inverters.return_value = 1 + + register_service() + + # Add assertions to verify the behavior + mock_dbus_service.assert_called_once() + + # Additional assertions + mock_dbus_service.assert_called_once_with( + servicename="mock_value", + paths=unittest.mock.ANY, + actual_inverter=0, + ) + +if __name__ == '__main__': + unittest.main() \ No newline at end of file From d21f513d8aaeb5ddd14d8066d88f7e9e1a63fde0 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:51:23 +0000 Subject: [PATCH 03/31] Refactor test_dbus-opendtu.py to remove unused mock_constants parameter in test_register_service method --- test_dbus-opendtu.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_dbus-opendtu.py b/test_dbus-opendtu.py index 601340b..927b90f 100644 --- a/test_dbus-opendtu.py +++ b/test_dbus-opendtu.py @@ -20,7 +20,7 @@ class TestRegisterService(unittest.TestCase): @patch('dbus_opendtu.get_config_value') @patch('dbus_opendtu.constants') - def test_register_service(self, mock_constants, mock_get_config_value, mock_dbus_service): + def test_register_service(self, mock_get_config_value, mock_dbus_service): def get_config_value_side_effect(key, *args, **kwargs): if isinstance(key, dict): key = key.get('key', 'mock_value') From 6c2c3a725745e7b0457cd77a450669d452fa833a Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 11 Nov 2024 16:20:16 +0000 Subject: [PATCH 04/31] since paths is used as constant I moved it --- constants.py | 39 +++++++++++++++++++++++++++++++++++++++ dbus_opendtu.py | 40 ---------------------------------------- dbus_service.py | 3 +-- 3 files changed, 40 insertions(+), 42 deletions(-) diff --git a/constants.py b/constants.py index 7abbbdf..7203ca9 100644 --- a/constants.py +++ b/constants.py @@ -1,5 +1,44 @@ '''Global constants''' +# region formatting helping functions (used in constant) +def _kwh(_p, value: float) -> str: + return f"{round(value, 2)}KWh" + +def _a(_p, value: float) -> str: + return f"{round(value, 1)}A" + +def _w(_p, value: float) -> str: + return f"{round(value, 1)}W" + +def _v(_p, value: float) -> str: + return f"{round(value, 1)}V" +# endregion + DTUVARIANT_AHOY = "ahoy" DTUVARIANT_OPENDTU = "opendtu" DTUVARIANT_TEMPLATE = "template" + + +VICTRON_PATHS = { + "/Ac/Energy/Forward": { + "initial": None, + "textformat": _kwh, + }, # energy produced by pv inverter + "/Ac/Power": {"initial": None, "textformat": _w}, + "/Ac/L1/Voltage": {"initial": None, "textformat": _v}, + "/Ac/L2/Voltage": {"initial": None, "textformat": _v}, + "/Ac/L3/Voltage": {"initial": None, "textformat": _v}, + "/Ac/L1/Current": {"initial": None, "textformat": _a}, + "/Ac/L2/Current": {"initial": None, "textformat": _a}, + "/Ac/L3/Current": {"initial": None, "textformat": _a}, + "/Ac/L1/Power": {"initial": None, "textformat": _w}, + "/Ac/L2/Power": {"initial": None, "textformat": _w}, + "/Ac/L3/Power": {"initial": None, "textformat": _w}, + "/Ac/L1/Energy/Forward": {"initial": None, "textformat": _kwh}, + "/Ac/L2/Energy/Forward": {"initial": None, "textformat": _kwh}, + "/Ac/L3/Energy/Forward": {"initial": None, "textformat": _kwh}, + "/Ac/Out/L1/I": {"initial": None, "textformat": _a}, + "/Ac/Out/L1/V": {"initial": None, "textformat": _v}, + "/Ac/Out/L1/P": {"initial": None, "textformat": _w}, + "/Dc/0/Voltage": {"initial": None, "textformat": _v}, +} \ No newline at end of file diff --git a/dbus_opendtu.py b/dbus_opendtu.py index 7ce244e..dc5bc71 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -58,43 +58,6 @@ def initialize(): logging.warning("NumberOfTemplates not set, using default") number_of_templates = 0 - # region formatting - def _kwh(_p, value: float) -> str: - return f"{round(value, 2)}KWh" - - def _a(_p, value: float) -> str: - return f"{round(value, 1)}A" - - def _w(_p, value: float) -> str: - return f"{round(value, 1)}W" - - def _v(_p, value: float) -> str: - return f"{round(value, 1)}V" - # endregion - - paths = { - "/Ac/Energy/Forward": { - "initial": None, - "textformat": _kwh, - }, # energy produced by pv inverter - "/Ac/Power": {"initial": None, "textformat": _w}, - "/Ac/L1/Voltage": {"initial": None, "textformat": _v}, - "/Ac/L2/Voltage": {"initial": None, "textformat": _v}, - "/Ac/L3/Voltage": {"initial": None, "textformat": _v}, - "/Ac/L1/Current": {"initial": None, "textformat": _a}, - "/Ac/L2/Current": {"initial": None, "textformat": _a}, - "/Ac/L3/Current": {"initial": None, "textformat": _a}, - "/Ac/L1/Power": {"initial": None, "textformat": _w}, - "/Ac/L2/Power": {"initial": None, "textformat": _w}, - "/Ac/L3/Power": {"initial": None, "textformat": _w}, - "/Ac/L1/Energy/Forward": {"initial": None, "textformat": _kwh}, - "/Ac/L2/Energy/Forward": {"initial": None, "textformat": _kwh}, - "/Ac/L3/Energy/Forward": {"initial": None, "textformat": _kwh}, - "/Ac/Out/L1/I": {"initial": None, "textformat": _a}, - "/Ac/Out/L1/V": {"initial": None, "textformat": _v}, - "/Ac/Out/L1/P": {"initial": None, "textformat": _w}, - "/Dc/0/Voltage": {"initial": None, "textformat": _v}, - } def register_service(): @@ -107,7 +70,6 @@ def register_service(): servicename = get_config_value(config, "Servicename", "INVERTER", 0, "com.victronenergy.pvinverter") service = DbusService( servicename=servicename, - paths=paths, actual_inverter=0, ) @@ -127,7 +89,6 @@ def register_service(): ) DbusService( servicename=servicename, - paths=paths, actual_inverter=actual_inverter + 1, ) @@ -142,7 +103,6 @@ def register_service(): ) service = DbusService( servicename=servicename, - paths=paths, actual_inverter=actual_template, istemplate=True, ) diff --git a/dbus_service.py b/dbus_service.py index bc7df0f..91ea68a 100644 --- a/dbus_service.py +++ b/dbus_service.py @@ -52,7 +52,6 @@ class DbusService: def __init__( self, servicename, - paths, actual_inverter, istemplate=False, ): @@ -95,7 +94,7 @@ def __init__( ) self._dbusservice = VeDbusService(f"{servicename}.http_{self.deviceinstance}", dbus_conn) - self._paths = paths + self._paths = constants.VICTRON_PATHS # Create the management objects, as specified in the ccgx dbus-api document self._dbusservice.add_path("/Mgmt/ProcessName", __file__) From 1e1190b628483d1059b4ba815bbaba16dfa091a6 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:20:42 +0000 Subject: [PATCH 05/31] Refactor imports in dbus_opendtu.py --- dbus_opendtu.py | 23 +---------------------- imports.py | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 22 deletions(-) create mode 100644 imports.py diff --git a/dbus_opendtu.py b/dbus_opendtu.py index dc5bc71..2790a7c 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -1,28 +1,7 @@ #!/usr/bin/env python '''module to read data from dtu/template and show in VenusOS''' -# File specific rules -# pylint: disable=broad-except - -# system imports: -import logging -import logging.handlers -import os -import configparser -import sys - -# our imports: -import constants -import tests -from helpers import * - -# Victron imports: -from dbus_service import DbusService - -if sys.version_info.major == 2: - import gobject # pylint: disable=E0401 -else: - from gi.repository import GLib as gobject # pylint: disable=E0401 +from imports import * config = None paths = None diff --git a/imports.py b/imports.py new file mode 100644 index 0000000..78e9867 --- /dev/null +++ b/imports.py @@ -0,0 +1,24 @@ +# imports.py +""" Imports for the project """ + +# pylint: disable=w0611 + +# system imports: +import logging +import logging.handlers +import os +import configparser +import sys + +# our imports: +import constants +import tests +from helpers import * + +# Victron imports: +from dbus_service import DbusService + +if sys.version_info.major == 2: + import gobject # pylint: disable=E0401 +else: + from gi.repository import GLib as gobject # pylint: disable=E0401 From 964265732a0ea21324fdbeec7928473b0908cba8 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:20:55 +0000 Subject: [PATCH 06/31] Refactor test_dbus-opendtu.py to remove unused mock_constants parameter in test_register_service method --- test_dbus-opendtu.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/test_dbus-opendtu.py b/test_dbus-opendtu.py index 927b90f..8e54f09 100644 --- a/test_dbus-opendtu.py +++ b/test_dbus-opendtu.py @@ -1,7 +1,8 @@ +""" Unit tests for the dbus_opendtu.py module """ + import unittest from unittest.mock import patch, MagicMock import sys -import os # Mocking the dbus and other dependencies before importing the module to test sys.modules['dbus'] = MagicMock() @@ -9,18 +10,17 @@ sys.modules['gi.repository'] = MagicMock() sys.modules['dbus.mainloop.glib'] = MagicMock() -# Add the directory containing dbus_opendtu.py to the Python path -sys.path.insert(0, os.path.abspath(os.path.dirname(__file__))) +from dbus_opendtu import register_service # pylint: disable=E0401,C0413 # noqa: E402 -from dbus_opendtu import register_service class TestRegisterService(unittest.TestCase): + """ Test cases for the register_service function """ + @patch('dbus_opendtu.config', {'DEFAULT': {'DTU': 'opendtu'}}) @patch('dbus_opendtu.DbusService') @patch('dbus_opendtu.get_config_value') - @patch('dbus_opendtu.constants') - def test_register_service(self, mock_get_config_value, mock_dbus_service): + """ Test the register_service function """ def get_config_value_side_effect(key, *args, **kwargs): if isinstance(key, dict): key = key.get('key', 'mock_value') @@ -31,7 +31,6 @@ def get_config_value_side_effect(key, *args, **kwargs): }.get(key, 'mock_value') mock_get_config_value.side_effect = get_config_value_side_effect - #mock_dbus_service.return_value = MagicMock() mock_dbus_service_instance = mock_dbus_service.return_value mock_dbus_service_instance.get_number_of_inverters.return_value = 1 @@ -43,9 +42,9 @@ def get_config_value_side_effect(key, *args, **kwargs): # Additional assertions mock_dbus_service.assert_called_once_with( servicename="mock_value", - paths=unittest.mock.ANY, actual_inverter=0, ) + if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() From 52e327528ad0f9f12951b703638485444b7e923c Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:22:05 +0000 Subject: [PATCH 07/31] Refactor code to remove unused paths --- dbus_opendtu.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dbus_opendtu.py b/dbus_opendtu.py index 2790a7c..2d399ff 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -4,14 +4,14 @@ from imports import * config = None -paths = None number_of_inverters = 0 number_of_templates = 0 + def initialize(): """ Initialize the module """ # pylint: disable=w0603 - global config, paths, number_of_inverters, number_of_templates + global config, number_of_inverters, number_of_templates # configure logging config = configparser.ConfigParser() @@ -38,7 +38,6 @@ def initialize(): number_of_templates = 0 - def register_service(): """ Register the service """ global number_of_inverters @@ -86,6 +85,7 @@ def register_service(): istemplate=True, ) + def main(): """ Main function """ initialize() @@ -107,5 +107,6 @@ def main(): except Exception as error: logging.critical("Error at %s", "main", exc_info=error) + if __name__ == "__main__": main() From f531c41bc378f65b589c000c72a1c7fd15519ea7 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:39:24 +0000 Subject: [PATCH 08/31] Refactor code to remove unused paths parameter in DbusService constructor --- tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests.py b/tests.py index 8a36f4d..5709f0d 100644 --- a/tests.py +++ b/tests.py @@ -158,7 +158,7 @@ def test_template_values(test_service): def run_tests(): '''function to run tests''' test_get_value_by_path() - test_service = DbusService(servicename="testing", paths="dummy", actual_inverter=0) + test_service = DbusService(servicename="testing", actual_inverter=0) test_opendtu_reachable(test_service) test_opendtu_producing(test_service) test_ahoy_values(test_service) From 5790d95b37b4a7c0316f67189867b9ab4ae6edc9 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:39:44 +0000 Subject: [PATCH 09/31] Refactor test_dbus-opendtu.py to use register_services instead of register_service --- test_dbus-opendtu.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test_dbus-opendtu.py b/test_dbus-opendtu.py index 8e54f09..d69fb91 100644 --- a/test_dbus-opendtu.py +++ b/test_dbus-opendtu.py @@ -10,13 +10,12 @@ sys.modules['gi.repository'] = MagicMock() sys.modules['dbus.mainloop.glib'] = MagicMock() -from dbus_opendtu import register_service # pylint: disable=E0401,C0413 # noqa: E402 +from dbus_opendtu import register_services # pylint: disable=E0401,C0413 # noqa: E402 class TestRegisterService(unittest.TestCase): """ Test cases for the register_service function """ - @patch('dbus_opendtu.config', {'DEFAULT': {'DTU': 'opendtu'}}) @patch('dbus_opendtu.DbusService') @patch('dbus_opendtu.get_config_value') def test_register_service(self, mock_get_config_value, mock_dbus_service): @@ -34,7 +33,7 @@ def get_config_value_side_effect(key, *args, **kwargs): mock_dbus_service_instance = mock_dbus_service.return_value mock_dbus_service_instance.get_number_of_inverters.return_value = 1 - register_service() + register_services({'DEFAULT': {'DTU': 'opendtu'}}) # Add assertions to verify the behavior mock_dbus_service.assert_called_once() From 0e4e41dc9830553875620861af411678a6212633 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:40:02 +0000 Subject: [PATCH 10/31] Refactor code to use register_services instead of register_service --- dbus_opendtu.py | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/dbus_opendtu.py b/dbus_opendtu.py index 2d399ff..1c4366d 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -3,16 +3,17 @@ from imports import * -config = None -number_of_inverters = 0 -number_of_templates = 0 +def getConfig(): + """ + Reads the configuration from a config.ini file and sets up logging. -def initialize(): - """ Initialize the module """ - # pylint: disable=w0603 - global config, number_of_inverters, number_of_templates + The function reads the configuration file located in the same directory as the script. + It configures the logging level based on the value specified in the configuration file. + Returns: + configparser.ConfigParser: The configuration object containing the parsed configuration. + """ # configure logging config = configparser.ConfigParser() config.read(f"{(os.path.dirname(os.path.realpath(__file__)))}/config.ini") @@ -23,6 +24,17 @@ def initialize(): level=logging_level, ) + return config + + +def register_services(config): + """ + Registers DTU devices and templates based on the configuration. + + Args: + config (configparser.ConfigParser): The configuration object containing the parsed configuration. + """ + try: number_of_inverters = int(config["DEFAULT"]["NumberOfInvertersToQuery"]) except (KeyError, ValueError) as ex: @@ -37,12 +49,12 @@ def initialize(): logging.warning("NumberOfTemplates not set, using default") number_of_templates = 0 + try: + dtuvariant = config["DEFAULT"]["DTU"] + except KeyError: + logging.critical("DTU key not found in configuration") + return -def register_service(): - """ Register the service """ - global number_of_inverters - - dtuvariant = config["DEFAULT"]["DTU"] if dtuvariant != constants.DTUVARIANT_TEMPLATE: logging.critical("Registering dtu devices") servicename = get_config_value(config, "Servicename", "INVERTER", 0, "com.victronenergy.pvinverter") @@ -88,7 +100,9 @@ def register_service(): def main(): """ Main function """ - initialize() + config = getConfig() + + # TODO: I think it is better to run the tests inside CI/CD pipeline instead of running it here tests.run_tests() try: @@ -99,12 +113,12 @@ def main(): # Have a mainloop, so we can send/receive asynchronous calls to and from dbus DBusGMainLoop(set_as_default=True) - register_service() + register_services(config) logging.info("Connected to dbus, and switching over to gobject.MainLoop() (= event based)") mainloop = gobject.MainLoop() mainloop.run() - except Exception as error: + except Exception as error: # pylint: disable=W0718 logging.critical("Error at %s", "main", exc_info=error) From a8e3014acf8d4f7e1bacec6594cebb0333500701 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:15:17 +0000 Subject: [PATCH 11/31] Refactor code to use get_DbusServices instead of register_services --- dbus_opendtu.py | 34 +++++++++++++++++++++++++--------- test_dbus-opendtu.py | 4 ++-- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/dbus_opendtu.py b/dbus_opendtu.py index 1c4366d..92d0647 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -27,14 +27,20 @@ def getConfig(): return config -def register_services(config): +def get_DbusServices(config): """ - Registers DTU devices and templates based on the configuration. + Retrieves and registers D-Bus services based on the provided configuration. Args: - config (configparser.ConfigParser): The configuration object containing the parsed configuration. + config (dict): Configuration dictionary containing the necessary settings. + + Returns: + list: A list of registered DbusService instances. """ + services = [] + + # region Get the configuration values try: number_of_inverters = int(config["DEFAULT"]["NumberOfInvertersToQuery"]) except (KeyError, ValueError) as ex: @@ -54,14 +60,17 @@ def register_services(config): except KeyError: logging.critical("DTU key not found in configuration") return + # endregion + # region Register the inverters if dtuvariant != constants.DTUVARIANT_TEMPLATE: - logging.critical("Registering dtu devices") + logging.info("Registering dtu devices") servicename = get_config_value(config, "Servicename", "INVERTER", 0, "com.victronenergy.pvinverter") service = DbusService( servicename=servicename, actual_inverter=0, ) + services.append(service) if number_of_inverters == 0: # pylint: disable=W0621 @@ -77,11 +86,13 @@ def register_services(config): actual_inverter + 1, "com.victronenergy.pvinverter" ) - DbusService( + services.append(DbusService( servicename=servicename, actual_inverter=actual_inverter + 1, - ) + )) + # endregion + # region Register the templates for actual_template in range(number_of_templates): logging.critical("Registering Templates") servicename = get_config_value( @@ -91,11 +102,14 @@ def register_services(config): actual_template, "com.victronenergy.pvinverter" ) - service = DbusService( + services.append(DbusService( servicename=servicename, actual_inverter=actual_template, istemplate=True, - ) + )) + # endregion + + return services def main(): @@ -113,7 +127,9 @@ def main(): # Have a mainloop, so we can send/receive asynchronous calls to and from dbus DBusGMainLoop(set_as_default=True) - register_services(config) + services = get_DbusServices(config) + + logging.info("Registered %d services", len(services)) logging.info("Connected to dbus, and switching over to gobject.MainLoop() (= event based)") mainloop = gobject.MainLoop() diff --git a/test_dbus-opendtu.py b/test_dbus-opendtu.py index d69fb91..ecfffd0 100644 --- a/test_dbus-opendtu.py +++ b/test_dbus-opendtu.py @@ -10,7 +10,7 @@ sys.modules['gi.repository'] = MagicMock() sys.modules['dbus.mainloop.glib'] = MagicMock() -from dbus_opendtu import register_services # pylint: disable=E0401,C0413 # noqa: E402 +from dbus_opendtu import get_DbusServices # pylint: disable=E0401,C0413 # noqa: E402 class TestRegisterService(unittest.TestCase): @@ -33,7 +33,7 @@ def get_config_value_side_effect(key, *args, **kwargs): mock_dbus_service_instance = mock_dbus_service.return_value mock_dbus_service_instance.get_number_of_inverters.return_value = 1 - register_services({'DEFAULT': {'DTU': 'opendtu'}}) + get_DbusServices({'DEFAULT': {'DTU': 'opendtu'}}) # Add assertions to verify the behavior mock_dbus_service.assert_called_once() From bbf7e7da3286fde27af92cbba864f997905ca5de Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:16:25 +0000 Subject: [PATCH 12/31] Refactor logging statement to use info level instead of critical --- dbus_opendtu.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbus_opendtu.py b/dbus_opendtu.py index 92d0647..2b08e07 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -120,7 +120,7 @@ def main(): tests.run_tests() try: - logging.critical("Start") + logging.info("Start") from dbus.mainloop.glib import DBusGMainLoop # pylint: disable=E0401,C0415 From d2ac513d118610693be635038d60e94008e78ae5 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:42:43 +0000 Subject: [PATCH 13/31] move code to add sign_of_life_all_services function and use it to send a 'sign of life' signal to all services --- dbus_opendtu.py | 25 ++++++++++++++++++++++++- dbus_service.py | 14 ++------------ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/dbus_opendtu.py b/dbus_opendtu.py index 2b08e07..e3fa0d4 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -112,9 +112,27 @@ def get_DbusServices(config): return services +def sign_of_life_all_services(services): + """ + Sends a 'sign of life' signal to all services in the provided list. + + Args: + services (list): A list of service objects. Each service object must have a 'sign_of_life' method. + + Returns: + bool: Always returns True to keep the timeout active. + """ + for service in services: + service.sign_of_life() + return True + + def main(): """ Main function """ config = getConfig() + signofliveinterval = int(get_config_value(config, "SignOfLifeLog", "DEFAULT", "", 1)) + + logging.debug("SignOfLifeLog: %d", signofliveinterval) # TODO: I think it is better to run the tests inside CI/CD pipeline instead of running it here tests.run_tests() @@ -128,9 +146,14 @@ def main(): DBusGMainLoop(set_as_default=True) services = get_DbusServices(config) - logging.info("Registered %d services", len(services)) + # Use a single timeout to call sign_of_life for all services + gobject.timeout_add(signofliveinterval * 60 * 1000, sign_of_life_all_services, services) + + # Use another timeout to update all services + # gobject.timeout_add(1000, update_all_services, services) # Update every 1000 ms + logging.info("Connected to dbus, and switching over to gobject.MainLoop() (= event based)") mainloop = gobject.MainLoop() mainloop.run() diff --git a/dbus_service.py b/dbus_service.py index 91ea68a..6be2524 100644 --- a/dbus_service.py +++ b/dbus_service.py @@ -94,7 +94,7 @@ def __init__( ) self._dbusservice = VeDbusService(f"{servicename}.http_{self.deviceinstance}", dbus_conn) - self._paths = constants.VICTRON_PATHS + self._paths = constants.VICTRON_PATHS # Create the management objects, as specified in the ccgx dbus-api document self._dbusservice.add_path("/Mgmt/ProcessName", __file__) @@ -143,7 +143,6 @@ def __init__( onchangecallback=self._handlechangedvalue, ) - gobject.timeout_add(self._get_sign_of_life_interval() * 60 * 1000, self._sign_of_life) gobject.timeout_add(self._get_polling_interval(), self._update) @staticmethod @@ -194,7 +193,6 @@ def _read_config_dtu(self, actual_inverter): {constants.DTUVARIANT_TEMPLATE}") self.deviceinstance = int(config[f"INVERTER{self.pvinverternumber}"]["DeviceInstance"]) self.acposition = int(get_config_value(config, "AcPosition", "INVERTER", self.pvinverternumber)) - self.signofliveinterval = get_config_value(config, "SignOfLifeLog", "DEFAULT", "", 1) self.useyieldday = int(get_config_value(config, "useYieldDay", "DEFAULT", "", 0)) self.pvinverterphase = str(config[f"INVERTER{self.pvinverternumber}"]["Phase"]) self.host = get_config_value(config, "Host", "INVERTER", self.pvinverternumber) @@ -235,7 +233,6 @@ def _read_config_template(self, template_number): self.deviceinstance = int(config[f"TEMPLATE{template_number}"]["DeviceInstance"]) self.customname = config[f"TEMPLATE{template_number}"]["Name"] self.acposition = int(config[f"TEMPLATE{template_number}"]["AcPosition"]) - self.signofliveinterval = get_config_value(config, "SignOfLifeLog", "DEFAULT", "", 1) self.useyieldday = int(get_config_value(config, "useYieldDay", "DEFAULT", "", 0)) self.pvinverterphase = str(config[f"TEMPLATE{template_number}"]["Phase"]) self.digestauth = is_true(get_config_value(config, "DigestAuth", "TEMPLATE", template_number, False)) @@ -337,13 +334,6 @@ def _get_polling_interval(self): polling_interval = self.pollinginterval return polling_interval - def _get_sign_of_life_interval(self): - '''Get intervall in seconds how often sign of life logs should be created.''' - value = self.signofliveinterval - if not value: - value = 0 - return int(value) - def _get_status_url(self): if self.dtuvariant == constants.DTUVARIANT_OPENDTU: url = self.get_opendtu_base_url() + "/livedata/status" @@ -527,7 +517,7 @@ def get_ts_last_success(self, meter_data): '''return ts_last_success from the meter_data structure - depending on the API version''' return meter_data["inverter"][self.pvinverternumber]["ts_last_success"] - def _sign_of_life(self): + def sign_of_life(self): logging.debug("Last inverter #%d _update() call: %s", self.pvinverternumber, self._last_update) logging.info("Last inverter #%d '/Ac/Power': %s", self.pvinverternumber, self._dbusservice["/Ac/Power"]) return True From f806c56357040d2de3d7c617aacbe477631facad Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:45:30 +0000 Subject: [PATCH 14/31] Refactor file paths to use underscore instead of hyphen in dbus_opendtu.py and restart.sh --- restart.sh | 2 +- service/run | 2 +- uninstall.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/restart.sh b/restart.sh index 267131a..c061077 100644 --- a/restart.sh +++ b/restart.sh @@ -1,3 +1,3 @@ #!/bin/bash SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) -kill $(pgrep -f "python $SCRIPT_DIR/dbus-opendtu.py") +kill $(pgrep -f "python $SCRIPT_DIR/dbus_opendtu.py") diff --git a/service/run b/service/run index 057b79f..ee967be 100644 --- a/service/run +++ b/service/run @@ -9,4 +9,4 @@ if [ -f "$FILENAME" ]; then #If the file exists, print its contents to stdout cat "$FILENAME" fi -python $(realpath $SCRIPT_DIR/../dbus-opendtu.py) +python $(realpath $SCRIPT_DIR/../dbus_opendtu.py) diff --git a/uninstall.sh b/uninstall.sh index 9da9f63..4d50cef 100644 --- a/uninstall.sh +++ b/uninstall.sh @@ -10,7 +10,7 @@ if [ -d /service/$SERVICE_NAME ]; then fi # end the dbus-opendtu process -kill $(pgrep -f "python $SCRIPT_DIR/dbus-opendtu.py") +kill $(pgrep -f "python $SCRIPT_DIR/dbus_opendtu.py") # delete old logs if they exist if [ -f $SCRIPT_DIR/current.log ]; then From f7367b10db786442873b09cf0d95b618667c7a19 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Thu, 14 Nov 2024 19:25:51 +0000 Subject: [PATCH 15/31] remove dependency and move code the main function (update routine) --- dbus_opendtu.py | 25 ++++++++++++++++++++++++- dbus_service.py | 17 +++++------------ 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/dbus_opendtu.py b/dbus_opendtu.py index e3fa0d4..9333271 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -127,6 +127,29 @@ def sign_of_life_all_services(services): return True +def update_all_services(services): + """ + Updates all services in the provided list. + + Args: + services (list): A list of service objects. + Each service object must have an 'update' method and + a 'polling_interval' and a 'polling_last_polling' attribute. + + Returns: + bool: Always returns True to keep the timeout active. + """ + if sys.version_info.major == 2: + current_time = gobject.get_current_time() + else: + current_time = gobject.get_real_time() // 1000 + for service in services: + if current_time - service.last_polling >= service.polling_interval: + service.update() + service.last_polling = current_time + return True + + def main(): """ Main function """ config = getConfig() @@ -152,7 +175,7 @@ def main(): gobject.timeout_add(signofliveinterval * 60 * 1000, sign_of_life_all_services, services) # Use another timeout to update all services - # gobject.timeout_add(1000, update_all_services, services) # Update every 1000 ms + gobject.timeout_add(1000, update_all_services, services) logging.info("Connected to dbus, and switching over to gobject.MainLoop() (= event based)") mainloop = gobject.MainLoop() diff --git a/dbus_service.py b/dbus_service.py index 6be2524..d140f72 100644 --- a/dbus_service.py +++ b/dbus_service.py @@ -20,11 +20,6 @@ # victron imports: import dbus -if sys.version_info.major == 2: - import gobject -else: - from gi.repository import GLib as gobject - sys.path.insert( 1, os.path.join( @@ -143,7 +138,8 @@ def __init__( onchangecallback=self._handlechangedvalue, ) - gobject.timeout_add(self._get_polling_interval(), self._update) + self.polling_interval = self._get_polling_interval() + self.last_polling = 0 @staticmethod def get_ac_inverter_state(current): @@ -519,10 +515,11 @@ def get_ts_last_success(self, meter_data): def sign_of_life(self): logging.debug("Last inverter #%d _update() call: %s", self.pvinverternumber, self._last_update) - logging.info("Last inverter #%d '/Ac/Power': %s", self.pvinverternumber, self._dbusservice["/Ac/Power"]) + logging.info("[%s] Last inverter #%d '/Ac/Power': %s", self._servicename, + self.pvinverternumber, self._dbusservice["/Ac/Power"]) return True - def _update(self): + def update(self): logging.debug("_update") successful = False try: @@ -560,10 +557,6 @@ def _update(self): else: self.last_update_successful = False - # return true, otherwise add_timeout will be removed from GObject - see docs - # http://library.isr.ist.utl.pt/docs/pygtk2reference/gobject-functions.html#function-gobject--timeout-add - return True - def _update_index(self): if self.dry_run: return From 2bb1f0267aeb0c116f09f2666e4e77187eadf013 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Fri, 15 Nov 2024 14:49:01 +0000 Subject: [PATCH 16/31] Add constants instead of hardcoded values --- constants.py | 52 ++++++++++++++++++++++++++++--------------------- dbus_service.py | 30 ++++++++++++++++------------ 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/constants.py b/constants.py index 7203ca9..3e6405e 100644 --- a/constants.py +++ b/constants.py @@ -1,44 +1,52 @@ '''Global constants''' # region formatting helping functions (used in constant) + + def _kwh(_p, value: float) -> str: return f"{round(value, 2)}KWh" + def _a(_p, value: float) -> str: return f"{round(value, 1)}A" + def _w(_p, value: float) -> str: return f"{round(value, 1)}W" + def _v(_p, value: float) -> str: return f"{round(value, 1)}V" # endregion + DTUVARIANT_AHOY = "ahoy" DTUVARIANT_OPENDTU = "opendtu" DTUVARIANT_TEMPLATE = "template" +PRODUCTNAME = "henne49_dbus-opendtu" +CONNECTION = "TCP/IP (HTTP)" VICTRON_PATHS = { - "/Ac/Energy/Forward": { - "initial": None, - "textformat": _kwh, - }, # energy produced by pv inverter - "/Ac/Power": {"initial": None, "textformat": _w}, - "/Ac/L1/Voltage": {"initial": None, "textformat": _v}, - "/Ac/L2/Voltage": {"initial": None, "textformat": _v}, - "/Ac/L3/Voltage": {"initial": None, "textformat": _v}, - "/Ac/L1/Current": {"initial": None, "textformat": _a}, - "/Ac/L2/Current": {"initial": None, "textformat": _a}, - "/Ac/L3/Current": {"initial": None, "textformat": _a}, - "/Ac/L1/Power": {"initial": None, "textformat": _w}, - "/Ac/L2/Power": {"initial": None, "textformat": _w}, - "/Ac/L3/Power": {"initial": None, "textformat": _w}, - "/Ac/L1/Energy/Forward": {"initial": None, "textformat": _kwh}, - "/Ac/L2/Energy/Forward": {"initial": None, "textformat": _kwh}, - "/Ac/L3/Energy/Forward": {"initial": None, "textformat": _kwh}, - "/Ac/Out/L1/I": {"initial": None, "textformat": _a}, - "/Ac/Out/L1/V": {"initial": None, "textformat": _v}, - "/Ac/Out/L1/P": {"initial": None, "textformat": _w}, - "/Dc/0/Voltage": {"initial": None, "textformat": _v}, -} \ No newline at end of file + "/Ac/Energy/Forward": { + "initial": None, + "textformat": _kwh, + }, # energy produced by pv inverter + "/Ac/Power": {"initial": None, "textformat": _w}, + "/Ac/L1/Voltage": {"initial": None, "textformat": _v}, + "/Ac/L2/Voltage": {"initial": None, "textformat": _v}, + "/Ac/L3/Voltage": {"initial": None, "textformat": _v}, + "/Ac/L1/Current": {"initial": None, "textformat": _a}, + "/Ac/L2/Current": {"initial": None, "textformat": _a}, + "/Ac/L3/Current": {"initial": None, "textformat": _a}, + "/Ac/L1/Power": {"initial": None, "textformat": _w}, + "/Ac/L2/Power": {"initial": None, "textformat": _w}, + "/Ac/L3/Power": {"initial": None, "textformat": _w}, + "/Ac/L1/Energy/Forward": {"initial": None, "textformat": _kwh}, + "/Ac/L2/Energy/Forward": {"initial": None, "textformat": _kwh}, + "/Ac/L3/Energy/Forward": {"initial": None, "textformat": _kwh}, + "/Ac/Out/L1/I": {"initial": None, "textformat": _a}, + "/Ac/Out/L1/V": {"initial": None, "textformat": _v}, + "/Ac/Out/L1/P": {"initial": None, "textformat": _w}, + "/Dc/0/Voltage": {"initial": None, "textformat": _v}, +} diff --git a/dbus_service.py b/dbus_service.py index d140f72..32b68a2 100644 --- a/dbus_service.py +++ b/dbus_service.py @@ -3,6 +3,8 @@ # File specific rules # pylint: disable=broad-except, import-error, wrong-import-order, wrong-import-position +# region [Imports] + # system imports: import configparser import os @@ -29,16 +31,26 @@ ) from vedbus import VeDbusService # noqa - must be placed after the sys.path.insert +# endregion + + +class DbusServiceRegistry(type): + """ + Metaclass for registering and iterating over D-Bus services. -class PvInverterRegistry(type): - '''Run a registry for all PV Inverter''' + This metaclass maintains a registry of D-Bus services and provides an iterator + to iterate over the registered services. + + Methods: + __iter__(cls): Returns an iterator over the registered D-Bus services. + """ def __iter__(cls): return iter(cls._registry) class DbusService: '''Main class to register PV Inverter in DBUS''' - __metaclass__ = PvInverterRegistry + __metaclass__ = DbusServiceRegistry _registry = [] _meter_data = None _test_meter_data = None @@ -51,11 +63,6 @@ def __init__( istemplate=False, ): - # This is (for now) not used elsewhere and is more of a constant - # than a contstuctor attribute - productname = "henne49_dbus-opendtu" - connection = "TCP/IP (HTTP)" - if servicename == "testing": self.max_age_ts = 600 self.pvinverternumber = actual_inverter @@ -63,7 +70,6 @@ def __init__( return self._registry.append(self) - self._last_update = 0 self._servicename = servicename self.last_update_successful = False @@ -79,7 +85,7 @@ def __init__( else: self._read_config_template(actual_inverter) - logging.critical("%s /DeviceInstance = %d", servicename, self.deviceinstance) + logging.info("%s /DeviceInstance = %d", servicename, self.deviceinstance) # Allow for multiple Instance per process in DBUS dbus_conn = ( @@ -95,12 +101,12 @@ def __init__( self._dbusservice.add_path("/Mgmt/ProcessName", __file__) self._dbusservice.add_path("/Mgmt/ProcessVersion", "Unkown version, and running on Python " + platform.python_version()) - self._dbusservice.add_path("/Mgmt/Connection", connection) + self._dbusservice.add_path("/Mgmt/Connection", constants.CONNECTION) # Create the mandatory objects self._dbusservice.add_path("/DeviceInstance", self.deviceinstance) self._dbusservice.add_path("/ProductId", 0xFFFF) # id assigned by Victron Support from SDM630v2.py - self._dbusservice.add_path("/ProductName", productname) + self._dbusservice.add_path("/ProductName", constants.PRODUCTNAME) self._dbusservice.add_path("/CustomName", self._get_name()) logging.info(f"Name of Inverters found: {self._get_name()}") self._dbusservice.add_path("/Connected", 1) From 6a43f116cc524b8e115747c9ce1e8d200a821e48 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Fri, 15 Nov 2024 14:49:22 +0000 Subject: [PATCH 17/31] move tests in proper folder --- tests/__init__.py | 0 test_dbus-opendtu.py => tests/test_dbus_opendtu.py | 4 ++++ test_helpers.py => tests/test_helpers.py | 4 ++++ 3 files changed, 8 insertions(+) create mode 100644 tests/__init__.py rename test_dbus-opendtu.py => tests/test_dbus_opendtu.py (91%) rename test_helpers.py => tests/test_helpers.py (98%) diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test_dbus-opendtu.py b/tests/test_dbus_opendtu.py similarity index 91% rename from test_dbus-opendtu.py rename to tests/test_dbus_opendtu.py index ecfffd0..f6e5b6c 100644 --- a/test_dbus-opendtu.py +++ b/tests/test_dbus_opendtu.py @@ -3,6 +3,10 @@ import unittest from unittest.mock import patch, MagicMock import sys +import os + +# Add the parent directory of dbus_opendtu to the system path +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) # Mocking the dbus and other dependencies before importing the module to test sys.modules['dbus'] = MagicMock() diff --git a/test_helpers.py b/tests/test_helpers.py similarity index 98% rename from test_helpers.py rename to tests/test_helpers.py index 6e3b905..596ed1b 100644 --- a/test_helpers.py +++ b/tests/test_helpers.py @@ -8,6 +8,10 @@ import unittest from unittest.mock import MagicMock import json + +# Add the parent directory of dbus_opendtu to the system path +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) # noqa pylint: disable=wrong-import-position + from helpers import ( get_config_value, get_default_config, From 1ca51d2d3aac84dae4c1f164b662f7888728d926 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Fri, 15 Nov 2024 16:20:57 +0000 Subject: [PATCH 18/31] move functions to helpers instead of constants; add unit tests and improve function --- .vscode/settings.json | 28 +++++++++------------------- constants.py | 20 +------------------- helpers.py | 18 ++++++++++++++++++ tests/test_helpers.py | 42 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 69 insertions(+), 39 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 76be030..0cdc348 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,20 +1,10 @@ { - "[python]": { - "editor.defaultFormatter": "ms-python.autopep8" - }, - "python.formatting.provider": "autopep8", - "autopep8.args": [ - "--max-line-length", - "120" - ], - "editor.formatOnSave": true, - "python.testing.unittestArgs": [ - "-v", - "-s", - ".", - "-p", - "test_*.py" - ], - "python.testing.pytestEnabled": false, - "python.testing.unittestEnabled": true, -} \ No newline at end of file + "[python]": { + "editor.defaultFormatter": "ms-python.autopep8" + }, + "autopep8.args": ["--max-line-length", "120"], + "editor.formatOnSave": true, + "python.testing.pytestEnabled": false, + "python.testing.unittestEnabled": true, + "python.testing.unittestArgs": ["-v", "-s", "./tests", "-p", "test_*"] +} diff --git a/constants.py b/constants.py index 3e6405e..6d7232f 100644 --- a/constants.py +++ b/constants.py @@ -1,24 +1,6 @@ '''Global constants''' -# region formatting helping functions (used in constant) - - -def _kwh(_p, value: float) -> str: - return f"{round(value, 2)}KWh" - - -def _a(_p, value: float) -> str: - return f"{round(value, 1)}A" - - -def _w(_p, value: float) -> str: - return f"{round(value, 1)}W" - - -def _v(_p, value: float) -> str: - return f"{round(value, 1)}V" -# endregion - +from helpers import _kwh, _a, _w, _v DTUVARIANT_AHOY = "ahoy" DTUVARIANT_OPENDTU = "opendtu" diff --git a/helpers.py b/helpers.py index b342e14..9c2d4a7 100644 --- a/helpers.py +++ b/helpers.py @@ -11,6 +11,24 @@ import logging +# region formatting helping functions (used in constant) +def _kwh(_p, value: float) -> str: + return f"{value:.2f}KWh" + + +def _a(_p, value: float) -> str: + return f"{value:.1f}A" + + +def _w(_p, value: float) -> str: + return f"{value:.1f}W" + + +def _v(_p, value: float) -> str: + return f"{value:.1f}V" +# endregion + + def get_config_value(config, name, inverter_or_template, inverter_or_tpl_number, defaultvalue=None): '''check if config value exist in current inverter/template's section, otherwise throw error''' if name in config[f"{inverter_or_template}{inverter_or_tpl_number}"]: diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 596ed1b..727c2e8 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -19,7 +19,11 @@ convert_to_expected_type, get_ahoy_field_by_name, is_true, - timeit + timeit, + _kwh, + _a, + _w, + _v, ) sys.modules['vedbus'] = MagicMock() sys.modules['dbus'] = MagicMock() @@ -228,6 +232,42 @@ def test_part_get_values_for_inverts(self): self.assertEqual(voltage, 225.66) self.assertEqual(current, None) + def test_kwh(self): + ''' Test the _kwh() function. ''' + self.assertEqual(_kwh(None, 123.456), "123.46KWh") + self.assertEqual(_kwh(None, 1.234), "1.23KWh") + self.assertEqual(_kwh(None, -1.234), "-1.23KWh") + self.assertEqual(_kwh(None, 0), "0.00KWh") + self.assertEqual(_kwh(None, 0.1234), "0.12KWh") + self.assertEqual(_kwh(None, 1.5678), "1.57KWh") + + def test_a(self): + ''' Test the _a() function. ''' + self.assertEqual(_a(None, 0), "0.0A") + self.assertEqual(_a(None, 0.45), "0.5A") + self.assertEqual(_a(None, 0.459), "0.5A") + self.assertEqual(_a(None, 1.2345), "1.2A") + self.assertEqual(_a(None, 1.5678), "1.6A") + self.assertEqual(_a(None, -1.5678), "-1.6A") + + def test_w(self): + ''' Test the _w() function. ''' + self.assertEqual(_w(None, 0), "0.0W") + self.assertEqual(_w(None, 0.45), "0.5W") + self.assertEqual(_w(None, 0.459), "0.5W") + self.assertEqual(_w(None, 1.2345), "1.2W") + self.assertEqual(_w(None, 1.5678), "1.6W") + self.assertEqual(_w(None, -1.5678), "-1.6W") + + def test_v(self): + ''' Test the _v() function. ''' + self.assertEqual(_v(None, 0), "0.0V") + self.assertEqual(_v(None, 0.45), "0.5V") + self.assertEqual(_v(None, 0.459), "0.5V") + self.assertEqual(_v(None, 1.2345), "1.2V") + self.assertEqual(_v(None, 1.5678), "1.6V") + self.assertEqual(_v(None, -1.5678), "-1.6V") + if __name__ == '__main__': unittest.main() From 6370a8ee23dbc387050f90ff645d384682876516 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Sat, 16 Nov 2024 15:39:45 +0000 Subject: [PATCH 19/31] Add coverage configuration and script - Add .coveragerc file to control coverage.py and exclude certain directories from coverage analysis. - Add run_coverage.sh script to run unit tests with coverage and generate coverage report. --- .coveragerc | 5 +++++ run_coverage.sh | 4 ++++ 2 files changed, 9 insertions(+) create mode 100644 .coveragerc create mode 100755 run_coverage.sh diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000..8beb35a --- /dev/null +++ b/.coveragerc @@ -0,0 +1,5 @@ +# .coveragerc to control coverage.py +[run] +omit = + */dist-packages/* + tests/* \ No newline at end of file diff --git a/run_coverage.sh b/run_coverage.sh new file mode 100755 index 0000000..ab4bca0 --- /dev/null +++ b/run_coverage.sh @@ -0,0 +1,4 @@ +#!/bin/bash + +python3-coverage run -m unittest discover -s tests -p "test_*.py" +python3-coverage report -m \ No newline at end of file From 3645b52f3b618d0367e884b7a4876a4ae8e03139 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Sat, 16 Nov 2024 15:40:02 +0000 Subject: [PATCH 20/31] Refactor code and improve error handling Move functions to helpers instead of constants, add unit tests, and improve function to handle cases where there are no inverters or templates to query. Also, remove the dependency on running tests within the main function. --- dbus_opendtu.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dbus_opendtu.py b/dbus_opendtu.py index 9333271..47fad30 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -55,6 +55,11 @@ def get_DbusServices(config): logging.warning("NumberOfTemplates not set, using default") number_of_templates = 0 + # If there are no inverters or templates, return an empty list + if number_of_inverters == 0 and number_of_templates == 0: + logging.critical("No inverters or templates to query") + return services + try: dtuvariant = config["DEFAULT"]["DTU"] except KeyError: @@ -158,7 +163,7 @@ def main(): logging.debug("SignOfLifeLog: %d", signofliveinterval) # TODO: I think it is better to run the tests inside CI/CD pipeline instead of running it here - tests.run_tests() + # tests.run_tests() try: logging.info("Start") From a52c72d33d5130bb01610bcb45db01e7600398fd Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Sat, 16 Nov 2024 15:40:21 +0000 Subject: [PATCH 21/31] Add more tests --- tests/test_dbus_opendtu.py | 159 +++++++++++++++++++++++++++++++++---- 1 file changed, 142 insertions(+), 17 deletions(-) diff --git a/tests/test_dbus_opendtu.py b/tests/test_dbus_opendtu.py index f6e5b6c..02e51f0 100644 --- a/tests/test_dbus_opendtu.py +++ b/tests/test_dbus_opendtu.py @@ -1,9 +1,10 @@ """ Unit tests for the dbus_opendtu.py module """ import unittest -from unittest.mock import patch, MagicMock +from unittest.mock import patch, MagicMock, mock_open import sys import os +import configparser # Add the parent directory of dbus_opendtu to the system path sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) @@ -14,40 +15,164 @@ sys.modules['gi.repository'] = MagicMock() sys.modules['dbus.mainloop.glib'] = MagicMock() -from dbus_opendtu import get_DbusServices # pylint: disable=E0401,C0413 # noqa: E402 +from dbus_opendtu import get_DbusServices, getConfig # pylint: disable=E0401,C0413 # noqa: E402 -class TestRegisterService(unittest.TestCase): - """ Test cases for the register_service function """ + +class TestDbusOpendtu(unittest.TestCase): + """ Test cases for the dbus_opendtu module """ @patch('dbus_opendtu.DbusService') - @patch('dbus_opendtu.get_config_value') - def test_register_service(self, mock_get_config_value, mock_dbus_service): + def test_register_service(self, mock_dbus_service): """ Test the register_service function """ - def get_config_value_side_effect(key, *args, **kwargs): - if isinstance(key, dict): - key = key.get('key', 'mock_value') - return { - 'number_of_inverters': 2, - 'number_of_templates': 1, - 'DTU': 'openDTU' - }.get(key, 'mock_value') + config = { + "DEFAULT": { + "NumberOfInvertersToQuery": "1", + "NumberOfTemplates": "0", + "DTU": "openDTU" + }, + "INVERTER0": { + "Phase": "L1", + "DeviceInstance": "34", + "AcPosition": "1" + }, + } - mock_get_config_value.side_effect = get_config_value_side_effect mock_dbus_service_instance = mock_dbus_service.return_value mock_dbus_service_instance.get_number_of_inverters.return_value = 1 - get_DbusServices({'DEFAULT': {'DTU': 'opendtu'}}) + get_DbusServices(config) # Add assertions to verify the behavior mock_dbus_service.assert_called_once() # Additional assertions mock_dbus_service.assert_called_once_with( - servicename="mock_value", + servicename="com.victronenergy.pvinverter", actual_inverter=0, ) + @patch( + "builtins.open", + new_callable=mock_open, + read_data=( + "[DEFAULT]\n" + "Logging=INFO\n" + "NumberOfInvertersToQuery=1\n" + "NumberOfTemplates=1\n" + "DTU=some_dtu" + ) + ) + @patch("os.path.realpath") + def test_get_config(self, mock_realpath, mock_open): # pylint: disable=W0613 + """ Test the get_config function """ + # Mock the realpath to return a fixed path + mock_realpath.return_value = "../config.example" + + # Call the function + config = getConfig() + + # Verify the return type + self.assertIsInstance(config, configparser.ConfigParser) + + # Verify the content of the config + self.assertEqual(config["DEFAULT"]["Logging"], "INFO") + self.assertEqual(config["DEFAULT"]["NumberOfInvertersToQuery"], "1") + self.assertEqual(config["DEFAULT"]["NumberOfTemplates"], "1") + self.assertEqual(config["DEFAULT"]["DTU"], "some_dtu") + + @patch('dbus_opendtu.DbusService') + @patch('dbus_opendtu.get_config_value') + def test_get_dbus_services_with_inverters(self, mock_get_config_value, mock_dbus_service): + """ Test get_DbusServices with inverters """ + mock_get_config_value.side_effect = lambda config, key, section, index, default: f"mock_value_{index}" + mock_dbus_service_instance = mock_dbus_service.return_value + mock_dbus_service_instance.get_number_of_inverters.return_value = 2 + + config = { + "DEFAULT": { + "NumberOfInvertersToQuery": "2", + "NumberOfTemplates": "0", + "DTU": "openDTU" + } + } + + services = get_DbusServices(config) + + self.assertEqual(len(services), 2) + mock_dbus_service.assert_any_call(servicename="mock_value_0", actual_inverter=0) + mock_dbus_service.assert_any_call(servicename="mock_value_1", actual_inverter=1) + + @patch("dbus_opendtu.get_config_value") + @patch("dbus_opendtu.DbusService") + def test_get_dbus_services_with_templates(self, mock_dbus_service, mock_get_config_value): + """ Test get_DbusServices with templates """ + # Mock the get_config_value function to return specific values + def get_config_value_side_effect(config, key, section, index, default): + if key == "NumberOfInvertersToQuery": + return 2 # Return an integer for the number of inverters + return f"mock_value_{index}" + + mock_get_config_value.side_effect = get_config_value_side_effect + + # Mock the DbusService instance + mock_dbus_service_instance = mock_dbus_service.return_value # pylint: disable=W0612 + + # Create a mock config + config = MagicMock() + + # Call the function + services = get_DbusServices(config) + + # Add assertions to verify the behavior + self.assertIsInstance(services, list) + self.assertEqual(len(services), 2) + + @patch("dbus_opendtu.DbusService") + def test_get_dbus_services_with_no_inverters_or_templates(self, mock_dbus_service): + """ Test get_DbusServices with no inverters or templates """ + # Create a mock config with the required values + config = { + "DEFAULT": { + "NumberOfInvertersToQuery": "0", + "NumberOfTemplates": "0", + "DTU": "openDTU" + }, + "INVERTER0": {}, # Add the required key to avoid KeyError + "TEMPLATE0": {} # Add the required key to avoid KeyError + } + + # Mock the get_number_of_inverters method to return 0 + mock_dbus_service_instance = mock_dbus_service.return_value + mock_dbus_service_instance.get_number_of_inverters.return_value = 0 + + services = get_DbusServices(config) + + self.assertEqual(len(services), 0) + mock_dbus_service.assert_not_called() + + @patch('dbus_opendtu.DbusService') + @patch('dbus_opendtu.get_config_value') + def test_get_dbus_services_with_missing_dtu_key(self, mock_get_config_value, mock_dbus_service): + """ Test get_DbusServices with missing DTU key """ + mock_get_config_value.side_effect = lambda config, key, section, index, default: f"mock_value_{index}" + mock_dbus_service_instance = mock_dbus_service.return_value + + config = { + "DEFAULT": { + "NumberOfInvertersToQuery": "1", + "NumberOfTemplates": "1" + } + } + + services = get_DbusServices(config) + + self.assertIsNone(services) + mock_dbus_service.assert_not_called() + + if __name__ == '__main__': + unittest.main() + if __name__ == '__main__': unittest.main() From 180748445f9add752c23c5d03beee04515578fba Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Sun, 17 Nov 2024 12:24:33 +0000 Subject: [PATCH 22/31] Completed Coverage for dbus_opendtu.py: 96% --- run_coverage.sh | 4 +- tests/test_dbus_opendtu.py | 225 ++++++++++++++++++++++++++++++++++++- 2 files changed, 224 insertions(+), 5 deletions(-) diff --git a/run_coverage.sh b/run_coverage.sh index ab4bca0..a35a102 100755 --- a/run_coverage.sh +++ b/run_coverage.sh @@ -1,4 +1,6 @@ #!/bin/bash python3-coverage run -m unittest discover -s tests -p "test_*.py" -python3-coverage report -m \ No newline at end of file +python3-coverage html +#python3-coverage report -m +#zip -r htmlcov.zip htmlcov \ No newline at end of file diff --git a/tests/test_dbus_opendtu.py b/tests/test_dbus_opendtu.py index 02e51f0..e2809ea 100644 --- a/tests/test_dbus_opendtu.py +++ b/tests/test_dbus_opendtu.py @@ -1,7 +1,7 @@ """ Unit tests for the dbus_opendtu.py module """ import unittest -from unittest.mock import patch, MagicMock, mock_open +from unittest.mock import patch, MagicMock, mock_open, ANY import sys import os import configparser @@ -12,11 +12,21 @@ # Mocking the dbus and other dependencies before importing the module to test sys.modules['dbus'] = MagicMock() sys.modules['vedbus'] = MagicMock() +# Mock the gi.repository.GLib module +sys.modules['gi'] = MagicMock() sys.modules['gi.repository'] = MagicMock() +sys.modules['gi.repository.GLib'] = MagicMock() +sys.modules['gi.repository.GLib.MainLoop'] = MagicMock() sys.modules['dbus.mainloop.glib'] = MagicMock() -from dbus_opendtu import get_DbusServices, getConfig # pylint: disable=E0401,C0413 # noqa: E402 +from dbus_opendtu import ( # pylint: disable=E0401,C0413 + get_DbusServices, + getConfig, + sign_of_life_all_services, + update_all_services, + main +) # noqa class TestDbusOpendtu(unittest.TestCase): @@ -151,6 +161,29 @@ def test_get_dbus_services_with_no_inverters_or_templates(self, mock_dbus_servic self.assertEqual(len(services), 0) mock_dbus_service.assert_not_called() + @patch("dbus_opendtu.DbusService") + def test_get_config_with_invalid_NumberOfInverter_and_Template_values(self, mock_dbus_service): + """ Test get_DbusServices with invalid NumberOfInverter and NumberOfTemplate values """ + # Create a mock config with the required values + config = { + "DEFAULT": { + "NumberOfInvertersToQuery": "invalid", + "NumberOfTemplates": "invalid", + "DTU": "openDTU" + }, + "INVERTER0": {}, # Add the required key to avoid KeyError + "TEMPLATE0": {} # Add the required key to avoid KeyError + } + + # Mock the get_number_of_inverters method to return 0 + mock_dbus_service_instance = mock_dbus_service.return_value + mock_dbus_service_instance.get_number_of_inverters.return_value = 0 + + services = get_DbusServices(config) + + self.assertEqual(len(services), 0) + mock_dbus_service.assert_not_called() + @patch('dbus_opendtu.DbusService') @patch('dbus_opendtu.get_config_value') def test_get_dbus_services_with_missing_dtu_key(self, mock_get_config_value, mock_dbus_service): @@ -170,8 +203,192 @@ def test_get_dbus_services_with_missing_dtu_key(self, mock_get_config_value, moc self.assertIsNone(services) mock_dbus_service.assert_not_called() - if __name__ == '__main__': - unittest.main() + def test_sign_of_life_all_services(self): + """ Test sign_of_life_all_services with a list of mock services """ + # Create a list of mock services + mock_service_1 = MagicMock() + mock_service_2 = MagicMock() + services = [mock_service_1, mock_service_2] + + # Call the function + result = sign_of_life_all_services(services) + + # Verify that the sign_of_life method was called on each service + mock_service_1.sign_of_life.assert_called_once() + mock_service_2.sign_of_life.assert_called_once() + + # Verify the return value + self.assertTrue(result) + + def test_sign_of_life_all_services_with_empty_list(self): + """ Test sign_of_life_all_services with an empty list """ + services = [] + + # Call the function + result = sign_of_life_all_services(services) + + # Verify the return value + self.assertTrue(result) + + def test_sign_of_life_all_services_with_no_sign_of_life_method(self): + """ Test sign_of_life_all_services with services missing sign_of_life method """ + # Create a list of mock services, one without sign_of_life method + mock_service_1 = MagicMock() + mock_service_2 = MagicMock() + del mock_service_2.sign_of_life + services = [mock_service_1, mock_service_2] + + # Call the function and expect an AttributeError + with self.assertRaises(AttributeError): + sign_of_life_all_services(services) + + @patch('dbus_opendtu.gobject') + def test_update_all_services(self, mock_gobject): + """ Test update_all_services with valid services """ + # Mock the current time + mock_gobject.get_real_time.return_value = 2000000 + + # Create mock services + mock_service_1 = MagicMock() + mock_service_1.polling_interval = 1000 + mock_service_1.last_polling = 1000 + + mock_service_2 = MagicMock() + mock_service_2.polling_interval = 2000 + mock_service_2.last_polling = 1000 + + services = [mock_service_1, mock_service_2] + + # Call the function + result = update_all_services(services) + + # Verify that the update method was called on each service + mock_service_1.update.assert_called_once() + mock_service_2.update.assert_not_called() + + # Verify that the last_polling attribute was updated + self.assertEqual(mock_service_1.last_polling, 2000) + self.assertEqual(mock_service_2.last_polling, 1000) + + # Verify the return value + self.assertTrue(result) + + @patch('dbus_opendtu.gobject') + def test_update_all_services_with_no_update_needed(self, mock_gobject): + """ Test update_all_services when no update is needed """ + # Mock the current time + mock_gobject.get_real_time.return_value = 2000000 + + # Create mock services + mock_service_1 = MagicMock() + mock_service_1.polling_interval = 1000 + mock_service_1.last_polling = 1999 + + mock_service_2 = MagicMock() + mock_service_2.polling_interval = 2000 + mock_service_2.last_polling = 1999 + + services = [mock_service_1, mock_service_2] + + # Call the function + result = update_all_services(services) + + # Verify that the update method was not called on any service + mock_service_1.update.assert_not_called() + mock_service_2.update.assert_not_called() + + # Verify the return value + self.assertTrue(result) + + @patch('dbus_opendtu.gobject') + def test_update_all_services_with_empty_list(self, mock_gobject): + """ Test update_all_services with an empty list """ + services = [] + + # Call the function + result = update_all_services(services) + + # Verify the return value + self.assertTrue(result) + + @patch('dbus_opendtu.gobject') + def test_update_all_services_with_missing_attributes(self, mock_gobject): + """ Test update_all_services with services missing required attributes """ + # Mock the current time + mock_gobject.get_real_time.return_value = 2000000 + + # Create mock services, one missing required attributes + mock_service_1 = MagicMock() + mock_service_1.polling_interval = 1000 + mock_service_1.last_polling = 1000 + + mock_service_2 = MagicMock() + del mock_service_2.polling_interval + del mock_service_2.last_polling + + services = [mock_service_1, mock_service_2] + + # Call the function and expect an AttributeError + with self.assertRaises(AttributeError): + update_all_services(services) + + @patch('dbus_opendtu.getConfig') + @patch('dbus_opendtu.get_config_value') + @patch('dbus_opendtu.get_DbusServices') + @patch('dbus_opendtu.sign_of_life_all_services') + @patch('dbus_opendtu.update_all_services') + @patch('dbus_opendtu.gobject') + def test_main( + self, + mock_gobject, + mock_update_all_services, + mock_sign_of_life_all_services, + mock_get_dbus_services, + mock_get_config_value, + mock_get_config, + ): + """ Test the main function """ + # Mock the configuration + mock_config = MagicMock() + mock_get_config.return_value = mock_config + mock_get_config_value.return_value = 1 + + # Mock the services + mock_services = [MagicMock()] + mock_get_dbus_services.return_value = mock_services + + # Mock the timeout_add method + def timeout_add_mock(interval, callback, *args, **kwargs): + callback(*args, **kwargs) + return True + + mock_gobject.timeout_add.side_effect = timeout_add_mock + + # Call the main function + main() + + # Assertions to verify the behavior + mock_get_config.assert_called_once() + mock_get_dbus_services.assert_called_once_with(mock_config) + mock_update_all_services.assert_called_once_with(mock_services) + mock_sign_of_life_all_services.assert_called_once_with(mock_services) + mock_gobject.MainLoop.assert_called_once() + + @patch('dbus_opendtu.get_DbusServices') + @patch('dbus_opendtu.gobject') + @patch('dbus_opendtu.logging') + def test_main_exception( + self, + mock_logging, + mock_gobject, + mock_get_dbus_services, # pylint: disable=W0613 + ): + """ Test the main function with exception """ + # Mock gobject.MainLoop to raise an exception + mock_gobject.MainLoop.side_effect = Exception("Test exception") + + main() + mock_logging.critical.assert_called_once_with("Error at %s", "main", exc_info=ANY) if __name__ == '__main__': From 9d4dc241f312387af57eb32721daaa521f28c0ab Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Sun, 17 Nov 2024 20:22:30 +0000 Subject: [PATCH 23/31] Refactor _get_serial method in dbus_service.py --- dbus_service.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/dbus_service.py b/dbus_service.py index 32b68a2..f739b99 100644 --- a/dbus_service.py +++ b/dbus_service.py @@ -270,18 +270,20 @@ def _read_config_template(self, template_number): # get the Serialnumber def _get_serial(self, pvinverternumber): + meter_data = None + serial = None if self.dtuvariant in (constants.DTUVARIANT_AHOY, constants.DTUVARIANT_OPENDTU): meter_data = self._get_data() - if self.dtuvariant == constants.DTUVARIANT_AHOY: - if not meter_data["inverter"][pvinverternumber]["name"]: - raise ValueError("Response does not contain name") - serial = meter_data["inverter"][pvinverternumber]["serial"] + if self.dtuvariant == constants.DTUVARIANT_AHOY: + if not meter_data["inverter"][pvinverternumber]["name"]: + raise ValueError("Response does not contain name") + serial = meter_data["inverter"][pvinverternumber]["serial"] - elif self.dtuvariant == constants.DTUVARIANT_OPENDTU: - if not meter_data["inverters"][pvinverternumber]["serial"]: - raise ValueError("Response does not contain serial attribute try name") - serial = meter_data["inverters"][pvinverternumber]["serial"] + elif self.dtuvariant == constants.DTUVARIANT_OPENDTU: + if not meter_data["inverters"][pvinverternumber]["serial"]: + raise ValueError("Response does not contain serial attribute try name") + serial = meter_data["inverters"][pvinverternumber]["serial"] elif self.dtuvariant == constants.DTUVARIANT_TEMPLATE: serial = self.serial From ad73463236aa5d199e251607ee2515eb729f7b6d Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Sun, 17 Nov 2024 20:27:16 +0000 Subject: [PATCH 24/31] Refactor _get_name and _get_status_url methods in dbus_service.py --- dbus_service.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/dbus_service.py b/dbus_service.py index f739b99..461ba60 100644 --- a/dbus_service.py +++ b/dbus_service.py @@ -291,6 +291,9 @@ def _get_serial(self, pvinverternumber): return serial def _get_name(self): + if self.dtuvariant in (constants.DTUVARIANT_OPENDTU, constants.DTUVARIANT_AHOY): + meter_data = self._get_data() + meter_data = None if self.dtuvariant in (constants.DTUVARIANT_OPENDTU, constants.DTUVARIANT_AHOY): meter_data = self._get_data() if self.dtuvariant == constants.DTUVARIANT_AHOY: @@ -339,6 +342,7 @@ def _get_polling_interval(self): return polling_interval def _get_status_url(self): + url = None if self.dtuvariant == constants.DTUVARIANT_OPENDTU: url = self.get_opendtu_base_url() + "/livedata/status" elif self.dtuvariant == constants.DTUVARIANT_AHOY: @@ -522,12 +526,39 @@ def get_ts_last_success(self, meter_data): return meter_data["inverter"][self.pvinverternumber]["ts_last_success"] def sign_of_life(self): + """ + Logs the last update time and the AC power value of the inverter. + + This method logs a debug message with the last update time of the inverter + and an info message with the AC power value of the inverter. + + Returns: + bool: Always returns True. + """ logging.debug("Last inverter #%d _update() call: %s", self.pvinverternumber, self._last_update) logging.info("[%s] Last inverter #%d '/Ac/Power': %s", self._servicename, self.pvinverternumber, self._dbusservice["/Ac/Power"]) return True def update(self): + """ + Updates the data from the DTU (Data Transfer Unit) and sets the DBus values if the data is up-to-date. + + This method performs the following steps: + 1. Refreshes the data from the DTU. + 2. Checks if the data is up-to-date. + 3. If in dry run mode, logs that no data is sent. + 4. If not in dry run mode, sets the DBus values. + 5. Updates the index. + 6. Handles various exceptions that may occur during the update process: + - requests.exceptions.RequestException: Logs an HTTP error if the last update was successful. + - ValueError: Logs a value error if the last update was successful. + - Exception: Logs a general error if the last update was successful. + 7. Logs a recovery message if the update was successful after a previous failure. + + Attributes: + successful (bool): Indicates whether the update was successful. + """ logging.debug("_update") successful = False try: @@ -660,7 +691,7 @@ def set_dbus_values(self): logging.debug("---") else: # three-phase inverter: split total power equally over all three phases - if ("3P" == self.pvinverterphase): + if "3P" == self.pvinverterphase: powerthird = power/3 # Single Phase Voltage = (3-Phase Voltage) / (sqrt(3)) From 25104b3553adf898fc6272e25a96a1f470681b5d Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Sun, 17 Nov 2024 23:56:30 +0000 Subject: [PATCH 25/31] wip working fetch --- tests/test_dbus_service.py | 108 +++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 tests/test_dbus_service.py diff --git a/tests/test_dbus_service.py b/tests/test_dbus_service.py new file mode 100644 index 0000000..b0b2680 --- /dev/null +++ b/tests/test_dbus_service.py @@ -0,0 +1,108 @@ +''' This file contains the unit tests for the DbusService class. ''' + +import unittest +from unittest.mock import patch, MagicMock, Mock +from dbus_service import DbusService +import requests + + +def mocked_requests_get(url, params=None, **kwargs): + """ hiouj""" + class MockResponse: + def __init__(self, json_data, status_code): + self.json_data = json_data + self.status_code = status_code + + def json(self): + return self.json_data + + def raise_for_status(self): + if self.status_code != 200: + raise requests.exceptions.HTTPError(f"{self.status_code} Error") + + if url == 'http://localhost/api/live': + return MockResponse({"key1": "value1"}, 200) + elif url == 'http://someotherurl.com/anothertest.json': + return MockResponse({"key2": "value2"}, 200) + + return MockResponse(None, 404) + + +class TestDbusService(unittest.TestCase): + + @patch('dbus_service.VeDbusService') + @patch('dbus_service.dbus') + @patch('dbus_service.logging') + @patch('dbus_service.constants') + @patch('dbus_service.platform') + @patch('dbus_service.os') + def test_init_testing(self, mock_os, mock_platform, mock_constants, mock_logging, mock_dbus, mock_VeDbusService): + # Test the initialization with servicename "testing" + servicename = "testing" + actual_inverter = 1 + istemplate = False + + service = DbusService(servicename, actual_inverter, istemplate) + + self.assertEqual(service.max_age_ts, 600) + self.assertEqual(service.pvinverternumber, actual_inverter) + self.assertFalse(service.useyieldday) + + myconfig = { + "DEFAULT": { + "DTU": "ahoy", + }, + "INVERTER0": { + "Phase": "L1", + "DeviceInstance": "34", + "AcPosition": "1", + "Host": "localhost", + } + } + + @patch('dbus_service.DbusService._get_config', return_value=myconfig) + @patch('dbus_service.dbus') + @patch('dbus_service.logging') + @patch('dbus_service.requests.get', side_effect=mocked_requests_get) + def test_init_non_template(self, mock__get_config, mock_dbus, mock_logging, mock_get): + """ Test fetch_url with custom responses for different URLs """ + + servicename = "com.victronenergy.pvinverter" + actual_inverter = 0 + istemplate = False + + # Initialize the DbusService + + # with self.assertRaises(ValueError): + service = DbusService(servicename, actual_inverter, istemplate) + + # Assertions to verify the behavior + self.assertEqual(service.dtuvariant, "ahoy") + + @patch('dbus_service.VeDbusService') + @patch('dbus_service.dbus') + @patch('dbus_service.logging') + @patch('dbus_service.constants') + @patch('dbus_service.platform') + @patch('dbus_service.os') + def test_init_template(self, mock_os, mock_platform, mock_constants, mock_logging, mock_dbus, mock_VeDbusService): + # Test the initialization with template servicename + servicename = "com.victronenergy.inverter" + actual_inverter = 1 + istemplate = True + + mock_os.environ = {} + mock_constants.VICTRON_PATHS = {} + mock_constants.CONNECTION = "connection" + mock_constants.PRODUCTNAME = "productname" + + service = DbusService(servicename, actual_inverter, istemplate) + + self.assertEqual(service._servicename, servicename) + self.assertEqual(service.pvinverternumber, actual_inverter) + self.assertFalse(service.last_update_successful) + self.assertIsNotNone(service._dbusservice) + + +if __name__ == '__main__': + unittest.main() From dc112822ca2772699c7fbfd6238deb208e08acd9 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 18 Nov 2024 13:28:14 +0000 Subject: [PATCH 26/31] Change number of Inverters in ahoy_0.5.93_live.json --- docs/ahoy_0.5.93_live.json | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/docs/ahoy_0.5.93_live.json b/docs/ahoy_0.5.93_live.json index bff9690..a924ebb 100644 --- a/docs/ahoy_0.5.93_live.json +++ b/docs/ahoy_0.5.93_live.json @@ -34,14 +34,7 @@ "Efficiency", "Q_AC" ], - "fld_units": [ - "V", - "A", - "W", - "Wh", - "kWh", - "%" - ], + "fld_units": ["V", "A", "W", "Wh", "kWh", "%"], "fld_names": [ "U_DC", "I_DC", @@ -50,16 +43,5 @@ "YieldTotal", "Irradiation" ], - "iv": [ - true, - true, - true, - false, - false, - false, - false, - false, - false, - false - ] + "iv": [true, true, false, false, false, false, false, false, false, false] } From 671e87f76e2044ccd9686605d1de7fb6dcacd3c2 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 18 Nov 2024 13:28:43 +0000 Subject: [PATCH 27/31] Added template and inverter tests --- tests/test_dbus_service.py | 148 ++++++++++++++++++++++++++++--------- 1 file changed, 113 insertions(+), 35 deletions(-) diff --git a/tests/test_dbus_service.py b/tests/test_dbus_service.py index b0b2680..3483875 100644 --- a/tests/test_dbus_service.py +++ b/tests/test_dbus_service.py @@ -1,52 +1,111 @@ ''' This file contains the unit tests for the DbusService class. ''' import unittest -from unittest.mock import patch, MagicMock, Mock -from dbus_service import DbusService +from unittest.mock import patch +import os +import json import requests +from dbus_service import DbusService + + +def mocked_requests_get(url, params=None, **kwargs): # pylint: disable=unused-argument + """ + Mock function to simulate `requests.get` behavior for specific URLs. + Args: + url (str): The URL to send the GET request to. + params (dict, optional): Dictionary of URL parameters to append to the URL. + **kwargs: Additional arguments passed to the request. -def mocked_requests_get(url, params=None, **kwargs): - """ hiouj""" + Returns: + MockResponse: A mock response object with predefined JSON data and status code. + + Raises: + requests.exceptions.HTTPError: If the status code of the response is not 200. + + Mocked URLs and their corresponding JSON files: + - 'http://localhost/api/live': Returns data from 'ahoy_0.5.93_live.json'. + - 'http://localhost/api/inverter/id/0': Returns data from 'ahoy_0.5.93_inverter-id-0.json'. + - 'http://localhost/api/inverter/id/1': Returns data from 'ahoy_0.5.93_inverter-id-1.json'. + - 'http://localhost/cm?cmnd=STATUS+8': Returns data from 'tasmota_shelly_2pm.json'. + - Any other URL: Returns a 404 status code. + """ class MockResponse: + """ + MockResponse is a mock class to simulate HTTP responses for testing purposes. + + Attributes: + json_data (dict): The JSON data to be returned by the mock response. + status_code (int): The HTTP status code of the mock response. + + Methods: + json(): Returns the JSON data of the mock response. + raise_for_status(): Raises an HTTPError if the status code is not 200. + """ + def __init__(self, json_data, status_code): self.json_data = json_data self.status_code = status_code def json(self): + """ + Returns the JSON data. + + Returns: + dict: The JSON data. + """ return self.json_data def raise_for_status(self): + """ + Raises an HTTPError if the HTTP request returned an unsuccessful status code. + + This method checks the status code of the HTTP response. If the status code is not 200, + it raises an HTTPError with a message containing the status code. + + Raises: + requests.exceptions.HTTPError: If the status code is not 200. + """ if self.status_code != 200: raise requests.exceptions.HTTPError(f"{self.status_code} Error") - if url == 'http://localhost/api/live': - return MockResponse({"key1": "value1"}, 200) - elif url == 'http://someotherurl.com/anothertest.json': - return MockResponse({"key2": "value2"}, 200) + print("Mock URL: ", url) + if url == 'http://localhost/api/live': + json_file_path = os.path.join(os.path.dirname(__file__), '../docs/ahoy_0.5.93_live.json') + with open(json_file_path, 'r', encoding="UTF-8") as file: + json_data = json.load(file) + return MockResponse(json_data, 200) + elif url == 'http://localhost/api/inverter/id/0': + json_file_path = os.path.join(os.path.dirname(__file__), '../docs/ahoy_0.5.93_inverter-id-0.json') + with open(json_file_path, 'r', encoding="UTF-8") as file: + json_data = json.load(file) + return MockResponse(json_data, 200) + elif url == 'http://localhost/api/inverter/id/1': + json_file_path = os.path.join(os.path.dirname(__file__), '../docs/ahoy_0.5.93_inverter-id-1.json') + with open(json_file_path, 'r', encoding="UTF-8") as file: + json_data = json.load(file) + return MockResponse(json_data, 200) + elif url == 'http://localhost/cm?cmnd=STATUS+8': + json_file_path = os.path.join(os.path.dirname(__file__), '../docs/tasmota_shelly_2pm.json') + with open(json_file_path, 'r', encoding="UTF-8") as file: + json_data = json.load(file) + return MockResponse(json_data, 200) return MockResponse(None, 404) class TestDbusService(unittest.TestCase): + """ Test the DbusService class """ - @patch('dbus_service.VeDbusService') @patch('dbus_service.dbus') - @patch('dbus_service.logging') - @patch('dbus_service.constants') - @patch('dbus_service.platform') - @patch('dbus_service.os') - def test_init_testing(self, mock_os, mock_platform, mock_constants, mock_logging, mock_dbus, mock_VeDbusService): - # Test the initialization with servicename "testing" - servicename = "testing" - actual_inverter = 1 + def test_init_testing(self, _mock_dbus): + """ Test the initialization of the DbusService class """ + servicename = "Nuclear_plant" + actual_inverter = -1 istemplate = False - service = DbusService(servicename, actual_inverter, istemplate) - - self.assertEqual(service.max_age_ts, 600) - self.assertEqual(service.pvinverternumber, actual_inverter) - self.assertFalse(service.useyieldday) + with self.assertRaises(KeyError): + DbusService(servicename, actual_inverter, istemplate) myconfig = { "DEFAULT": { @@ -79,23 +138,42 @@ def test_init_non_template(self, mock__get_config, mock_dbus, mock_logging, mock # Assertions to verify the behavior self.assertEqual(service.dtuvariant, "ahoy") - @patch('dbus_service.VeDbusService') - @patch('dbus_service.dbus') - @patch('dbus_service.logging') - @patch('dbus_service.constants') - @patch('dbus_service.platform') - @patch('dbus_service.os') - def test_init_template(self, mock_os, mock_platform, mock_constants, mock_logging, mock_dbus, mock_VeDbusService): + template_config = { + "DEFAULT": { + "DTU": "ahoy", + }, + "TEMPLATE0": { + "Username": "", + "Password": "", + "DigestAuth": "False", + "Host": "localhost", + "CUST_SN": "12345678", + "CUST_API_PATH": "cm?cmnd=STATUS+8", + "CUST_POLLING": "2000", + "CUST_Total": "StatusSNS/ENERGY/Total", + "CUST_Total_Mult": "1", + "CUST_Power": "StatusSNS/ENERGY/Power", + "CUST_Power_Mult": "1", + "CUST_Voltage": "StatusSNS/ENERGY/Voltage", + "CUST_Current": "StatusSNS/ENERGY/Current", + "Phase": "L1", + "DeviceInstance": "47", + "AcPosition": "1", + "Name": "Tasmota", + "Servicename": "com.victronenergy.grid" + } + } + + @ patch('dbus_service.DbusService._get_config', return_value=template_config) + @ patch('dbus_service.dbus') + @ patch('dbus_service.logging') + @patch('dbus_service.requests.get', side_effect=mocked_requests_get) + def test_init_template(self, mock__get_config, mock_dbus, mock_logging, mock_get): # Test the initialization with template servicename servicename = "com.victronenergy.inverter" - actual_inverter = 1 + actual_inverter = 0 istemplate = True - mock_os.environ = {} - mock_constants.VICTRON_PATHS = {} - mock_constants.CONNECTION = "connection" - mock_constants.PRODUCTNAME = "productname" - service = DbusService(servicename, actual_inverter, istemplate) self.assertEqual(service._servicename, servicename) From dda573111287d0e21ed26c54dc375e0024cfe925 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 18 Nov 2024 20:24:20 +0000 Subject: [PATCH 28/31] Move early fail condition to the right position --- dbus_opendtu.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dbus_opendtu.py b/dbus_opendtu.py index 47fad30..9796be7 100644 --- a/dbus_opendtu.py +++ b/dbus_opendtu.py @@ -55,11 +55,6 @@ def get_DbusServices(config): logging.warning("NumberOfTemplates not set, using default") number_of_templates = 0 - # If there are no inverters or templates, return an empty list - if number_of_inverters == 0 and number_of_templates == 0: - logging.critical("No inverters or templates to query") - return services - try: dtuvariant = config["DEFAULT"]["DTU"] except KeyError: @@ -81,6 +76,11 @@ def get_DbusServices(config): # pylint: disable=W0621 number_of_inverters = service.get_number_of_inverters() + # If there are no inverters or templates, return an empty list + if number_of_inverters == 0 and number_of_templates == 0: + logging.critical("No inverters or templates to query") + return [] # Empty list + if number_of_inverters > 1: # start our main-service if there are more than 1 inverter for actual_inverter in range(number_of_inverters - 1): From ee69fd35b4d3aad36352712b50db3a684577db57 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 18 Nov 2024 20:26:18 +0000 Subject: [PATCH 29/31] Refactored _read_config_dtu method and removed unreachable code - Removed unreachable code in the _read_config_dtu method. - Clarified the logic for reading the DTU variant and other configuration values. - Ensured that the DTU variant is either constants.DTUVARIANT_OPENDTU or constants.DTUVARIANT_AHOY, raising an error otherwise. - Improved readability and maintainability of the code. --- dbus_service.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/dbus_service.py b/dbus_service.py index 461ba60..24b33d8 100644 --- a/dbus_service.py +++ b/dbus_service.py @@ -191,8 +191,7 @@ def _read_config_dtu(self, actual_inverter): if self.dtuvariant not in (constants.DTUVARIANT_OPENDTU, constants.DTUVARIANT_AHOY): raise ValueError(f"Error in config.ini: DTU must be one of \ {constants.DTUVARIANT_OPENDTU}, \ - {constants.DTUVARIANT_AHOY}, \ - {constants.DTUVARIANT_TEMPLATE}") + {constants.DTUVARIANT_AHOY}") self.deviceinstance = int(config[f"INVERTER{self.pvinverternumber}"]["DeviceInstance"]) self.acposition = int(get_config_value(config, "AcPosition", "INVERTER", self.pvinverternumber)) self.useyieldday = int(get_config_value(config, "useYieldDay", "DEFAULT", "", 0)) @@ -309,10 +308,8 @@ def get_number_of_inverters(self): meter_data = self._get_data() if self.dtuvariant == constants.DTUVARIANT_AHOY: numberofinverters = len(meter_data["inverter"]) - elif self.dtuvariant == constants.DTUVARIANT_OPENDTU: + else: # Assuming the only other option is constants.DTUVARIANT_OPENDTU numberofinverters = len(meter_data["inverters"]) - else: - numberofinverters = 1 logging.info("Number of Inverters found: %s", numberofinverters) return numberofinverters From c6c094f556ac55bb00c773351b0fb937791c1740 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 18 Nov 2024 20:26:34 +0000 Subject: [PATCH 30/31] adjust tests --- tests/test_dbus_opendtu.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_dbus_opendtu.py b/tests/test_dbus_opendtu.py index e2809ea..64b0d18 100644 --- a/tests/test_dbus_opendtu.py +++ b/tests/test_dbus_opendtu.py @@ -159,7 +159,7 @@ def test_get_dbus_services_with_no_inverters_or_templates(self, mock_dbus_servic services = get_DbusServices(config) self.assertEqual(len(services), 0) - mock_dbus_service.assert_not_called() + mock_dbus_service.assert_called_once() # called once to check if there are inverters @patch("dbus_opendtu.DbusService") def test_get_config_with_invalid_NumberOfInverter_and_Template_values(self, mock_dbus_service): @@ -182,7 +182,7 @@ def test_get_config_with_invalid_NumberOfInverter_and_Template_values(self, mock services = get_DbusServices(config) self.assertEqual(len(services), 0) - mock_dbus_service.assert_not_called() + mock_dbus_service.assert_called_once() # called once to check if there are inverters @patch('dbus_opendtu.DbusService') @patch('dbus_opendtu.get_config_value') From dff4727718bb7a9071e43642fe1e59c7179d1473 Mon Sep 17 00:00:00 2001 From: 0x7878 <17197791+0x7878@users.noreply.github.com> Date: Mon, 18 Nov 2024 20:27:36 +0000 Subject: [PATCH 31/31] Add test to verify that number_of_inverters are set - Added a new test case to verify that the number of inverters is correctly set in the DbusService. - Configured the test with a specific DTU variant and number of inverters. - Used mocking to simulate the configuration and requests. - Included assertions to validate the expected behavior. --- tests/test_dbus_service.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_dbus_service.py b/tests/test_dbus_service.py index 3483875..d4b6a31 100644 --- a/tests/test_dbus_service.py +++ b/tests/test_dbus_service.py @@ -138,6 +138,35 @@ def test_init_non_template(self, mock__get_config, mock_dbus, mock_logging, mock # Assertions to verify the behavior self.assertEqual(service.dtuvariant, "ahoy") + config_for_test_if_number_of_inverters_are_set = { + "DEFAULT": { + "DTU": "ahoy", + "NumberOfInvertersToQuery": 0 + }, + "INVERTER0": { + "Phase": "L1", + "DeviceInstance": "34", + "AcPosition": "1", + "Host": "localhost", + }, + } + + @patch('dbus_service.DbusService._get_config', return_value=config_for_test_if_number_of_inverters_are_set) + @patch('dbus_service.dbus') + @patch('dbus_service.logging') + @patch('dbus_service.requests.get', side_effect=mocked_requests_get) + def test_if_number_of_inverters_are_set(self, mock__get_config, mock_dbus, mock_logging, mock_get): + """ Test fetch_url with custom responses for different URLs """ + + servicename = "com.victronenergy.pvinverter" + actual_inverter = 0 + istemplate = False + + service = DbusService(servicename, actual_inverter, istemplate) + + self.assertEqual(service.dtuvariant, "ahoy") + self.assertEqual(service.get_number_of_inverters(), 2) + template_config = { "DEFAULT": { "DTU": "ahoy",