From c1dd70f0654058ddae03fc1de6f452f8a967c26c Mon Sep 17 00:00:00 2001 From: "Dr.-Ing. Amilcar do Carmo Lucas" Date: Sat, 4 Apr 2026 18:14:14 +0200 Subject: [PATCH] test(data_model_vehicle_components): Added more tests --- .../data_model_vehicle_components_import.py | 2 +- tests/test_battery_cell_voltages.py | 78 --- ...st_data_model_vehicle_components_common.py | 28 + ...t_data_model_vehicle_components_display.py | 20 +- ...st_data_model_vehicle_components_import.py | 525 ++++++++++++++++++ ...ta_model_vehicle_components_json_schema.py | 63 +++ ...data_model_vehicle_components_templates.py | 28 + ...ata_model_vehicle_components_validation.py | 480 ++++++++++++++++ tests/unit_battery_cell_voltages.py | 107 ++++ ...unit_data_model_vehicle_components_base.py | 166 ++++++ ...t_data_model_vehicle_components_display.py | 178 ++++++ ...it_data_model_vehicle_components_import.py | 66 +++ ...ta_model_vehicle_components_json_schema.py | 274 +++++++++ 13 files changed, 1918 insertions(+), 97 deletions(-) delete mode 100755 tests/test_battery_cell_voltages.py create mode 100755 tests/unit_battery_cell_voltages.py create mode 100755 tests/unit_data_model_vehicle_components_display.py diff --git a/ardupilot_methodic_configurator/data_model_vehicle_components_import.py b/ardupilot_methodic_configurator/data_model_vehicle_components_import.py index 0b859bc2c..6c2a95b32 100644 --- a/ardupilot_methodic_configurator/data_model_vehicle_components_import.py +++ b/ardupilot_methodic_configurator/data_model_vehicle_components_import.py @@ -328,7 +328,7 @@ def _set_esc_type_from_fc_parameters(self, fc_parameters: dict[str, float], doc: self.set_component_value(("ESC", "FC Connection", "Type"), "AIO") if "MOT_PWM_TYPE" in doc and "values" in doc["MOT_PWM_TYPE"]: - protocol = str(doc["MOT_PWM_TYPE"]["values"].get(str(mot_pwm_type))) + protocol = doc["MOT_PWM_TYPE"]["values"].get(str(mot_pwm_type), "") if protocol: self.set_component_value(("ESC", "FC Connection", "Protocol"), protocol) # Fallback to MOT_PWM_TYPE_DICT if doc is not available diff --git a/tests/test_battery_cell_voltages.py b/tests/test_battery_cell_voltages.py deleted file mode 100755 index f7d86d900..000000000 --- a/tests/test_battery_cell_voltages.py +++ /dev/null @@ -1,78 +0,0 @@ -#!/usr/bin/env python3 - -""" -Tests for the battery_cell_voltages.py file. - -This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator - -SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas - -SPDX-License-Identifier: GPL-3.0-or-later -""" - -import unittest -from math import isnan - -from ardupilot_methodic_configurator.battery_cell_voltages import BatteryCell, _recommended_battery_cell_voltages - - -class TestBatteryCell(unittest.TestCase): # pylint: disable=missing-class-docstring - def test_chemistries(self) -> None: - expected_chemistries = ("LiIon", "LiIonSS", "LiIonSSHV", "Lipo", "LipoHV", "LipoHVSS", "NiCd", "NiMH") - chemistries = BatteryCell.chemistries() - assert chemistries == expected_chemistries - - def test_limit_max_voltage(self) -> None: - assert BatteryCell.limit_max_voltage("LiIon") == 4.2 - assert BatteryCell.limit_max_voltage("LipoHV") == 4.35 - assert BatteryCell.limit_max_voltage("NonExistentChemistry") == 4.45 - - def test_limit_min_voltage(self) -> None: - assert BatteryCell.limit_min_voltage("LiIon") == 2.5 - assert BatteryCell.limit_min_voltage("LipoHV") == 3.0 - assert BatteryCell.limit_min_voltage("NonExistentChemistry") == 1.0 - - def test_recommended_max_voltage(self) -> None: - assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell max") == 4.1 - assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell max")) - - def test_recommended_low_voltage(self) -> None: - assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell low") == 3.1 - assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell low")) - - def test_recommended_crit_voltage(self) -> None: - assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell crit") == 2.8 - assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell crit")) - - def test_recommended_arm_voltage(self) -> None: - assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell arm") == 3.6 - assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell arm")) - - def test_recommended_min_voltage(self) -> None: - assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell min") == 2.7 - assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell min")) - - def test_voltage_monoticity(self) -> None: - for chem in BatteryCell.chemistries(): - with self.subTest(chemistry=chem): - assert BatteryCell.limit_max_voltage(chem) == _recommended_battery_cell_voltages[chem].get("absolute_max") - assert BatteryCell.limit_min_voltage(chem) == _recommended_battery_cell_voltages[chem].get("absolute_min") - assert BatteryCell.limit_max_voltage(chem) >= BatteryCell.recommended_cell_voltage(chem, "Volt per cell max") - assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell max") >= BatteryCell.recommended_cell_voltage( - chem, "Volt per cell arm" - ) - assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell arm") >= BatteryCell.recommended_cell_voltage( - chem, "Volt per cell low" - ) - assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell low") >= BatteryCell.recommended_cell_voltage( - chem, "Volt per cell crit" - ) - # This is not required, the user might want to stop PID scaling above the critical voltage - # assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell crit") >= - # BatteryCell.recommended_cell_voltage(chem, "Volt per cell min") - assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell min") >= BatteryCell.limit_min_voltage(chem) - assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell crit") >= BatteryCell.limit_min_voltage(chem) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/test_data_model_vehicle_components_common.py b/tests/test_data_model_vehicle_components_common.py index d6aadbe24..fd0c90288 100755 --- a/tests/test_data_model_vehicle_components_common.py +++ b/tests/test_data_model_vehicle_components_common.py @@ -12,8 +12,10 @@ import copy from typing import Any, Optional, TypeVar +from unittest.mock import MagicMock from ardupilot_methodic_configurator.backend_filesystem_vehicle_components import VehicleComponents +from ardupilot_methodic_configurator.data_model_vehicle_components_display import ComponentDataModelDisplay from ardupilot_methodic_configurator.data_model_vehicle_components_json_schema import VehicleComponentsJsonSchema # Type variables for generic fixture factories @@ -163,6 +165,14 @@ } +def make_fc_schema(fc_body: dict[str, Any], *, definitions: Optional[dict[str, Any]] = None) -> dict[str, Any]: + """Build a minimal schema dict wrapping a single Flight Controller component body.""" + schema: dict[str, Any] = {"properties": {"Components": {"properties": {"Flight Controller": fc_body}}}} + if definitions is not None: + schema["definitions"] = definitions + return schema + + class ComponentDataModelFixtures: """Factory class for creating component data model fixtures.""" @@ -254,6 +264,24 @@ def create_realistic_model(model_class: type[T]) -> T: post_init({}) return model + @staticmethod + def create_mock_schema() -> MagicMock: + """Create a mock schema with default non-optional behavior.""" + schema = MagicMock() + schema.get_component_property_description.return_value = ("Test description", False) + return schema + + @staticmethod + def create_display_model_with_mock_schema(mock_schema: MagicMock) -> ComponentDataModelDisplay: + """Create a ComponentDataModelDisplay instance backed by a mock schema.""" + initial_data: dict[str, Any] = {"Components": {}, "Format version": 1} + component_datatypes: dict[str, Any] = {"Flight Controller": {"Product": {"Manufacturer": str}}} + schema_dict = ComponentDataModelFixtures.create_simple_schema() + schema = VehicleComponentsJsonSchema(schema_dict) + model = ComponentDataModelDisplay(initial_data, component_datatypes, schema) + model.schema = mock_schema + return model + class CommonAssertions: """Common assertion helpers for component data model tests.""" diff --git a/tests/test_data_model_vehicle_components_display.py b/tests/test_data_model_vehicle_components_display.py index 8410d9954..f4f50949e 100755 --- a/tests/test_data_model_vehicle_components_display.py +++ b/tests/test_data_model_vehicle_components_display.py @@ -16,7 +16,6 @@ from test_data_model_vehicle_components_common import ComponentDataModelFixtures from ardupilot_methodic_configurator.data_model_vehicle_components_display import ComponentDataModelDisplay -from ardupilot_methodic_configurator.data_model_vehicle_components_json_schema import VehicleComponentsJsonSchema # pylint: disable=redefined-outer-name @@ -24,28 +23,13 @@ @pytest.fixture def mock_schema() -> MagicMock: """Fixture providing a mock schema for display testing.""" - schema = MagicMock() - - # Default behavior: non-optional components - schema.get_component_property_description.return_value = ("Test description", False) - - return schema + return ComponentDataModelFixtures.create_mock_schema() @pytest.fixture def display_model(mock_schema) -> ComponentDataModelDisplay: """Fixture providing a ComponentDataModelDisplay instance for testing.""" - # Create minimal required dependencies - initial_data = {"Components": {}, "Format version": 1} - component_datatypes = {"Flight Controller": {"Product": {"Manufacturer": str}}} - schema_dict = ComponentDataModelFixtures.create_simple_schema() - schema = VehicleComponentsJsonSchema(schema_dict) - - # Create and configure the display model - model = ComponentDataModelDisplay(initial_data, component_datatypes, schema) - # Override with mock schema for easier testing - model.schema = mock_schema - return model + return ComponentDataModelFixtures.create_display_model_with_mock_schema(mock_schema) @pytest.fixture diff --git a/tests/test_data_model_vehicle_components_import.py b/tests/test_data_model_vehicle_components_import.py index ab3e4c0dd..b796e3c97 100755 --- a/tests/test_data_model_vehicle_components_import.py +++ b/tests/test_data_model_vehicle_components_import.py @@ -10,6 +10,7 @@ SPDX-License-Identifier: GPL-3.0-or-later """ +from math import nan from typing import Any from unittest.mock import patch @@ -20,6 +21,7 @@ RealisticDataTestMixin, ) +import ardupilot_methodic_configurator.data_model_vehicle_components_import as _import_module from ardupilot_methodic_configurator.battery_cell_voltages import BatteryCell from ardupilot_methodic_configurator.data_model_vehicle_components_import import ( BatteryVoltageSpecs, @@ -1762,3 +1764,526 @@ def test_estimate_cell_count_returns_zero_for_all_invalid_volt_per_cell(self, ba # Assert assert estimated == 0 + + +class TestComponentDataModelImportUncoveredBranches: + """Tests targeting previously uncovered branches in ComponentDataModelImport.""" + + @pytest.fixture + def basic_model(self) -> ComponentDataModelImport: + """Create a basic model with zero volt-per-cell specs.""" + return ComponentDataModelFixtures.create_basic_model(ComponentDataModelImport) + + @pytest.fixture + def realistic_model(self) -> ComponentDataModelImport: + """Create a realistic model with real battery specs.""" + return ComponentDataModelFixtures.create_realistic_model(ComponentDataModelImport) + + # ------------------------------------------------------------------ + # _reverse_key_search + # ------------------------------------------------------------------ + def test_system_logs_error_when_reverse_key_search_values_not_found(self) -> None: + """ + _reverse_key_search returns fallbacks when no values match and logs an error. + + GIVEN: A doc dict where none of the requested values exist in the param's 'values' entry + WHEN: _reverse_key_search is called + THEN: The fallback values should be returned AND an error should be logged + """ + doc = {"MY_PARAM": {"values": {"0": "None", "1": "MAVLink1"}}} + values_to_find = ["DroneCAN"] # not in the dict + fallbacks = [99] + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_error") as mock_err: + result = ComponentDataModelImport._reverse_key_search(doc, "MY_PARAM", values_to_find, fallbacks) + + assert result == fallbacks + mock_err.assert_called() + + def test_system_logs_error_when_reverse_key_search_length_differs(self) -> None: + """ + _reverse_key_search logs an error when len(values) != len(fallbacks). + + GIVEN: A values list and a fallbacks list of different lengths + WHEN: _reverse_key_search is called with a matching entry + THEN: An error should be logged about the length mismatch + AND: The found keys should still be returned + """ + doc = {"MY_PARAM": {"values": {"0": "None", "1": "MAVLink1"}}} + values_to_find = ["MAVLink1"] # matches key "1" + fallbacks_wrong_length = [1, 2, 3] # length != 1 + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_error") as mock_err: + result = ComponentDataModelImport._reverse_key_search(doc, "MY_PARAM", values_to_find, fallbacks_wrong_length) + + assert result == [1] # found key for "MAVLink1" + mock_err.assert_called() + + # ------------------------------------------------------------------ + # _verify_dict_is_uptodate with bitmask and invalid bit position + # ------------------------------------------------------------------ + def test_system_warns_and_skips_invalid_bitmask_bit_positions(self, basic_model) -> None: + """ + _verify_dict_is_uptodate warns and continues when a bitmask key is not an integer. + + GIVEN: A doc dict with a Bitmask entry that has a non-integer bit position key + WHEN: _verify_dict_is_uptodate is called + THEN: A warning should be logged for the invalid key + AND: Processing should continue for the remaining keys + """ + doc = {"RC_PROTOCOLS": {"Bitmask": {"not_a_number": "All", "9": "CRSF"}}} + dict_to_check = {"512": {"protocol": "CRSF"}} # key 512 = 2^9 + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning") as mock_warn: + _result = basic_model._verify_dict_is_uptodate(doc, dict_to_check, "RC_PROTOCOLS", "Bitmask") + + # The non-integer "not_a_number" key hits the except branch → "Invalid bit position" warning + mock_warn.assert_any_call("Invalid bit position %s in %s metadata", "not_a_number", "RC_PROTOCOLS") + + # ------------------------------------------------------------------ + # _set_battery_type_from_fc_parameters - I2C edge cases + # ------------------------------------------------------------------ + def test_system_warns_when_i2c_bus_index_is_out_of_range(self, realistic_model) -> None: + """ + System warns and defaults to first I2C bus when BATT_I2C_BUS is out of range. + + GIVEN: A BATT_MONITOR configured for I2C (type 5 = Solo) with BATT_I2C_BUS=99 + WHEN: _set_battery_type_from_fc_parameters is called + THEN: A warning should be logged about the out-of-range value + AND: The first I2C bus should be used as default + """ + fc_parameters: dict[str, Any] = {"BATT_MONITOR": 5, "BATT_I2C_BUS": 99} + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning") as mock_warn: + realistic_model._set_battery_type_from_fc_parameters(fc_parameters) + + texts = " ".join(str(c) for c in mock_warn.call_args_list) + assert "out of range" in texts or "BATT_I2C_BUS" in texts + + def test_system_warns_when_i2c_bus_value_is_not_an_integer(self, realistic_model) -> None: + """ + System warns and defaults to first I2C bus when BATT_I2C_BUS cannot be parsed as int. + + GIVEN: A BATT_MONITOR configured for I2C and BATT_I2C_BUS holds a non-integer string + WHEN: _set_battery_type_from_fc_parameters is called + THEN: A warning should be logged about the invalid value + """ + fc_parameters: dict[str, Any] = {"BATT_MONITOR": 5, "BATT_I2C_BUS": "not_an_int"} + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning") as mock_warn: + realistic_model._set_battery_type_from_fc_parameters(fc_parameters) + + texts = " ".join(str(c) for c in mock_warn.call_args_list) + assert "Invalid BATT_I2C_BUS" in texts or "defaulting" in texts + + def test_system_warns_when_batt_monitor_parameter_is_absent(self, basic_model) -> None: + """ + System logs a warning when BATT_MONITOR is absent from fc_parameters. + + GIVEN: fc_parameters that do not contain the BATT_MONITOR key + WHEN: _set_battery_type_from_fc_parameters is called + THEN: A warning should be logged about the missing parameter + """ + fc_parameters: dict[str, Any] = {} # no BATT_MONITOR key + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning") as mock_warn: + basic_model._set_battery_type_from_fc_parameters(fc_parameters) + + texts = " ".join(str(c) for c in mock_warn.call_args_list) + assert "BATT_MONITOR" in texts + + # ------------------------------------------------------------------ + # import_bat_voltage - invalid voltage_type + # ------------------------------------------------------------------ + def test_system_logs_error_for_invalid_voltage_type_in_import_bat_voltage(self, basic_model) -> None: + """ + import_bat_voltage logs an error and returns early for an unknown voltage_type. + + GIVEN: A BatteryVoltageSpecs with a valid param_name and a specs object + AND: voltage_type is not in BATTERY_CELL_VOLTAGE_TYPES + WHEN: import_bat_voltage is called + THEN: An error should be logged and no value should be stored + """ + specs = BatteryVoltageSpecs( + estimated_cell_count=4, + limit_min=3.0, + limit_max=4.5, + detected_chemistry="Lipo", + fc_parameters={"MOT_BAT_VOLT_MAX": 16.8}, + ) + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_error") as mock_err: + basic_model.import_bat_voltage(specs, "MOT_BAT_VOLT_MAX", "not_a_valid_type") + + mock_err.assert_called() + + # ------------------------------------------------------------------ + # _estimate_battery_cell_count - BATT_ARM_VOLT and MOT_BAT_VOLT_MIN fallbacks + # ------------------------------------------------------------------ + def test_system_uses_batt_arm_volt_as_fallback_for_cell_count_estimation(self, realistic_model) -> None: + """ + _estimate_battery_cell_count falls back to BATT_ARM_VOLT when higher-priority params absent. + + GIVEN: Only BATT_ARM_VOLT is present in fc_parameters (all higher-priority params absent) + WHEN: _estimate_battery_cell_count is called + THEN: The BATT_ARM_VOLT path should be taken as a fallback + AND: The result should be either a positive integer or 0 (if volt_per_cell is unset) + """ + # Only provide BATT_ARM_VOLT, omit MOT_BAT_VOLT_MAX/BATT_LOW_VOLT/BATT_CRT_VOLT + fc_parameters = {"BATT_ARM_VOLT": 15.2} + + with ( + patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning"), + patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_error"), + ): + result = realistic_model._estimate_battery_cell_count(fc_parameters) + + assert isinstance(result, int) + + def test_system_uses_mot_bat_volt_min_as_last_resort_for_cell_count_estimation(self, realistic_model) -> None: + """ + _estimate_battery_cell_count uses MOT_BAT_VOLT_MIN as the lowest-priority fallback. + + GIVEN: Only MOT_BAT_VOLT_MIN is present in fc_parameters + WHEN: _estimate_battery_cell_count is called + THEN: The MOT_BAT_VOLT_MIN path should be taken + AND: The result should be an integer (positive or 0) + """ + fc_parameters = {"MOT_BAT_VOLT_MIN": 12.8} + + with ( + patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning"), + patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_error"), + ): + result = realistic_model._estimate_battery_cell_count(fc_parameters) + + assert isinstance(result, int) + + # ------------------------------------------------------------------ + # _verify_dict_is_uptodate — protocols-match branch (line 250->259) + # and protocols-mismatch (line 279) + # ------------------------------------------------------------------ + def test_system_passes_when_code_and_doc_protocols_match(self, basic_model) -> None: + """ + _verify_dict_is_uptodate returns True when code protocol matches doc protocol. + + GIVEN: A doc and a dict_to_check where every protocol value agrees + WHEN: _verify_dict_is_uptodate is called + THEN: True is returned with no warning logged + AND: The False-branch of 'if code_protocol != doc_protocol' is exercised (250->259) + """ + doc = {"SERIAL1_PROTOCOL": {"values": {"1": "MAVLink1"}}} + dict_to_check = {"1": {"protocol": "MAVLink1"}} # matches exactly + + result = basic_model._verify_dict_is_uptodate(doc, dict_to_check, "SERIAL1_PROTOCOL", "values") + + assert result is True + + def test_system_warns_when_doc_key_is_absent_from_code_dict(self, basic_model) -> None: + """ + _verify_dict_is_uptodate logs a warning when a doc key is absent from the code dictionary. + + GIVEN: A doc with protocol key "99" that is not present in dict_to_check + WHEN: _verify_dict_is_uptodate is called + THEN: A warning is logged and False is returned (else-branch / line 279 path exercised) + """ + doc = {"SERIAL1_PROTOCOL": {"values": {"99": "FakeProtocol"}}} + dict_to_check = {"1": {"protocol": "MAVLink1"}} # "99" not present + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning") as mock_warn: + result = basic_model._verify_dict_is_uptodate(doc, dict_to_check, "SERIAL1_PROTOCOL", "values") + + assert result is False + mock_warn.assert_called() + + # ------------------------------------------------------------------ + # _set_serial_type_from_fc_parameters — skip protocol 0 and unknown (lines 275, 279) + # ------------------------------------------------------------------ + def test_system_skips_serial_port_with_protocol_zero(self, basic_model) -> None: + """ + _set_serial_type_from_fc_parameters skips a serial port whose protocol number is 0. + + GIVEN: fc_parameters with SERIAL1_PROTOCOL=0 (disabled) + WHEN: _set_serial_type_from_fc_parameters is called + THEN: No component values are changed for that port (continue at line 275 is exercised) + """ + fc_parameters: dict[str, Any] = {"SERIAL1_PROTOCOL": 0} + result = basic_model._set_serial_type_from_fc_parameters(fc_parameters) + assert isinstance(result, bool) + + def test_system_skips_serial_port_with_unknown_protocol_number(self, basic_model) -> None: + """ + _set_serial_type_from_fc_parameters skips a port when protocol number is not in the dict. + + GIVEN: fc_parameters with SERIAL1_PROTOCOL=999 (not in SERIAL_PROTOCOLS_DICT) + WHEN: _set_serial_type_from_fc_parameters is called + THEN: The port is silently skipped (continue at line 279 is exercised) + """ + fc_parameters: dict[str, Any] = {"SERIAL1_PROTOCOL": 999} + result = basic_model._set_serial_type_from_fc_parameters(fc_parameters) + assert isinstance(result, bool) + + # ------------------------------------------------------------------ + # _set_esc_type_from_fc_parameters — protocol empty (line 332) and dict fallback (line 335) + # ------------------------------------------------------------------ + def test_system_skips_setting_esc_protocol_when_doc_value_is_absent(self, basic_model) -> None: + """ + _set_esc_type_from_fc_parameters skips setting ESC protocol when mot_pwm_type absent from doc values. + + GIVEN: doc has MOT_PWM_TYPE.values but does NOT contain the key for mot_pwm_type=99 + WHEN: _set_esc_type_from_fc_parameters is called + THEN: No protocol is set via doc (if protocol: False branch at line 332 is exercised) + AND: The function completes without error + """ + fc_parameters: dict[str, Any] = {"MOT_PWM_TYPE": 99} # 99 not in doc + doc: dict[str, Any] = {"MOT_PWM_TYPE": {"values": {"0": "Normal", "6": "DShot600"}}} + + basic_model._set_esc_type_from_fc_parameters(fc_parameters, doc) + + # Should not crash; exact protocol value depends on model default + result = basic_model.get_component_value(("ESC", "FC Connection", "Protocol")) + assert result is not None + + def test_system_uses_mot_pwm_type_dict_fallback_when_doc_has_no_mot_pwm_type(self, basic_model) -> None: + """ + _set_esc_type_from_fc_parameters falls back to MOT_PWM_TYPE_DICT when doc lacks values. + + GIVEN: doc has no MOT_PWM_TYPE entry (empty) + AND: fc_parameters has MOT_PWM_TYPE=0 (which IS in MOT_PWM_TYPE_DICT as 'Normal') + WHEN: _set_esc_type_from_fc_parameters is called + THEN: Protocol is set to 'Normal' via the elif fallback (line 335 exercised) + """ + fc_parameters: dict[str, Any] = {"MOT_PWM_TYPE": 0} + doc: dict[str, Any] = {} # no MOT_PWM_TYPE in doc + + basic_model._set_esc_type_from_fc_parameters(fc_parameters, doc) + + result = basic_model.get_component_value(("ESC", "FC Connection", "Protocol")) + assert result == "Normal" + + # ------------------------------------------------------------------ + # _set_battery_type_from_fc_parameters — valid I2C bus index (line 363) + # and non-I2C list type (Analog) (line 376) + # ------------------------------------------------------------------ + def test_system_selects_i2c_bus_by_index_when_batt_i2c_bus_is_valid(self, basic_model) -> None: + """ + _set_battery_type_from_fc_parameters uses BATT_I2C_BUS index to select the I2C bus. + + GIVEN: BATT_MONITOR=5 (Solo, I2C type) and BATT_I2C_BUS=1 (selects I2C2 = index 1) + WHEN: _set_battery_type_from_fc_parameters is called + THEN: FC Connection Type should be set to 'I2C2' (fc_conn_type[1]) (line 363 exercised) + """ + fc_parameters: dict[str, Any] = {"BATT_MONITOR": 5, "BATT_I2C_BUS": 1} + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning"): + basic_model._set_battery_type_from_fc_parameters(fc_parameters) + + result = basic_model.get_component_value(("Battery Monitor", "FC Connection", "Type")) + assert result == "I2C2" + + def test_system_defaults_to_first_element_for_non_i2c_list_battery_type(self, basic_model) -> None: + """ + _set_battery_type_from_fc_parameters takes the first element of a non-I2C list type. + + GIVEN: BATT_MONITOR=3 (Analog Voltage Only) whose type is ANALOG_PORTS=['Analog'] + AND: That list is NOT a subset of I2C_PORTS + WHEN: _set_battery_type_from_fc_parameters is called + THEN: FC Connection Type should be set to 'Analog' (first element) (line 376 exercised) + """ + fc_parameters: dict[str, Any] = {"BATT_MONITOR": 3} + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning"): + basic_model._set_battery_type_from_fc_parameters(fc_parameters) + + result = basic_model.get_component_value(("Battery Monitor", "FC Connection", "Type")) + assert result == "Analog" + + # ------------------------------------------------------------------ + # _detect_battery_chemistry_from_voltages — isnan(score) log+continue (lines 504-509) + # ------------------------------------------------------------------ + def test_system_logs_warning_when_chemistry_score_is_nan(self, basic_model, monkeypatch) -> None: + """ + _detect_battery_chemistry_from_voltages logs a warning when chemistry_voltage_score returns NaN. + + GIVEN: BatteryCell.chemistry_voltage_score is patched to always return NaN + AND: fc_parameters contains MOT_BAT_VOLT_MAX=16.8 and current_chemistry='Lipo' + WHEN: _detect_battery_chemistry_from_voltages is called + THEN: A warning is logged for each parameter (lines 504-509 exercised) + AND: The function falls through to best_chemistry_for_voltage and returns a result + """ + monkeypatch.setattr( + BatteryCell, + "chemistry_voltage_score", + staticmethod(lambda *_a, **_kw: nan), + ) + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning") as mock_warn: + result = basic_model._detect_battery_chemistry_from_voltages({"MOT_BAT_VOLT_MAX": 16.8}, "Lipo") + + # NaN score causes warning and continue; best_chemistry_for_voltage then determines the result + assert mock_warn.called + assert result is None or isinstance(result, str) + + # ------------------------------------------------------------------ + # _set_gnss_type_from_fc_parameters — invalid connection type (lines 228-229) + # ------------------------------------------------------------------ + def test_system_logs_error_when_gnss_conn_type_is_neither_serial_nor_can(self, basic_model, monkeypatch) -> None: + """ + _set_gnss_type_from_fc_parameters logs an error for GPS types with unknown connection type. + + GIVEN: GNSS_RECEIVER_CONNECTION is patched so GPS_TYPE 99 maps to type=['USB'] (not SERIAL/CAN) + WHEN: _set_gnss_type_from_fc_parameters is called with fc_parameters={'GPS_TYPE': 99} + THEN: logging_error is called with 'Invalid GNSS connection type' (lines 228-229 exercised) + AND: FC Connection Type is set to 'None' + """ + fake_gnss_conn: dict[str, Any] = {"99": {"type": ["USB"], "protocol": "Unknown"}} + monkeypatch.setattr(_import_module, "GNSS_RECEIVER_CONNECTION", fake_gnss_conn) + + with patch.object(_import_module, "logging_error") as mock_err: + basic_model._set_gnss_type_from_fc_parameters({"GPS_TYPE": 99}) + + # Verify error was logged about invalid connection type + logged_text = " ".join(str(call) for call in mock_err.call_args_list) + assert "Invalid GNSS connection type" in logged_text + + result = basic_model.get_component_value(("GNSS Receiver", "FC Connection", "Type")) + assert result == "None" + + # ------------------------------------------------------------------ + # _verify_dict_is_uptodate — protocol mismatch (lines 141-142) + # ------------------------------------------------------------------ + def test_system_warns_when_code_and_doc_protocols_differ(self, basic_model) -> None: + """ + _verify_dict_is_uptodate logs a warning when check_key is found but protocols differ. + + GIVEN: A doc saying key "1" maps to "MAVLink2" + AND: dict_to_check saying key "1" maps to "MAVLink1" (mismatch!) + WHEN: _verify_dict_is_uptodate is called + THEN: A warning is logged about the protocol mismatch (lines 141-142 exercised) + AND: False is returned + """ + doc = {"SERIAL1_PROTOCOL": {"values": {"1": "MAVLink2"}}} # doc expects MAVLink2 + dict_to_check = {"1": {"protocol": "MAVLink1"}} # code has MAVLink1 — MISMATCH + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning") as mock_warn: + result = basic_model._verify_dict_is_uptodate(doc, dict_to_check, "SERIAL1_PROTOCOL", "values") + + assert result is False + logged_text = " ".join(str(c) for c in mock_warn.call_args_list) + assert "does not match" in logged_text or "MAVLink" in logged_text + + # ------------------------------------------------------------------ + # _set_serial_type_from_fc_parameters — RC_PROTOCOLS single bit not in dict (line 250->259) + # ------------------------------------------------------------------ + def test_system_skips_rc_protocol_assignment_when_single_bit_not_in_dict(self, basic_model) -> None: + """ + _set_serial_type_from_fc_parameters skips RC protocol assignment when bit value is not in dict. + + GIVEN: RC_PROTOCOLS = 131072 (2^17 = single bit set, but NOT in RC_PROTOCOLS_DICT which goes up to 2^16) + WHEN: _set_serial_type_from_fc_parameters is called + THEN: RC Receiver protocol is not updated (line 250->259 False branch exercised) + AND: The function completes without error + """ + fc_parameters: dict[str, Any] = {"RC_PROTOCOLS": 131072} # 2^17, not in dict + result = basic_model._set_serial_type_from_fc_parameters(fc_parameters) + assert isinstance(result, bool) + + # ------------------------------------------------------------------ + # _set_esc_type_from_fc_parameters — neither doc nor dict has the protocol (line 335->exit) + # ------------------------------------------------------------------ + def test_system_leaves_esc_protocol_unchanged_when_type_not_in_doc_or_dict(self, basic_model) -> None: + """ + _set_esc_type_from_fc_parameters leaves ESC protocol unchanged when mot_pwm_type is unknown. + + GIVEN: doc is empty (no MOT_PWM_TYPE key) + AND: fc_parameters has MOT_PWM_TYPE=999 (not in MOT_PWM_TYPE_DICT either) + WHEN: _set_esc_type_from_fc_parameters is called + THEN: Neither the doc branch nor the dict fallback sets the protocol (line 335->exit exercised) + AND: The function completes without error + """ + fc_parameters: dict[str, Any] = {"MOT_PWM_TYPE": 999} # not in dict + doc: dict[str, Any] = {} # no MOT_PWM_TYPE in doc either + + basic_model._set_esc_type_from_fc_parameters(fc_parameters, doc) + # No crash — function just falls through both if/elif conditions + + # ------------------------------------------------------------------ + # _set_esc_type_from_fc_parameters — MOT_PWM_TYPE non-integer (lines 318-320) + # ------------------------------------------------------------------ + def test_system_logs_error_for_non_integer_mot_pwm_type(self, basic_model) -> None: + """ + _set_esc_type_from_fc_parameters logs an error when MOT_PWM_TYPE cannot be converted to int. + + GIVEN: fc_parameters has MOT_PWM_TYPE='abc' (non-integer string) + WHEN: _set_esc_type_from_fc_parameters is called + THEN: An error is logged and mot_pwm_type defaults to 0 (lines 318-320 exercised) + """ + fc_parameters: dict[str, Any] = {"MOT_PWM_TYPE": "abc"} + doc: dict[str, Any] = {} + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_error") as mock_err: + basic_model._set_esc_type_from_fc_parameters(fc_parameters, doc) + + mock_err.assert_called() + + # ------------------------------------------------------------------ + # _set_battery_type_from_fc_parameters — I2C bus without BATT_I2C_BUS (line 376) + # ------------------------------------------------------------------ + def test_system_defaults_to_first_i2c_bus_when_batt_i2c_bus_not_provided(self, basic_model) -> None: + """ + _set_battery_type_from_fc_parameters defaults to I2C1 when BATT_I2C_BUS is absent. + + GIVEN: BATT_MONITOR=5 (Solo, I2C type) but NO BATT_I2C_BUS parameter + WHEN: _set_battery_type_from_fc_parameters is called + THEN: FC Connection Type defaults to 'I2C1' (first element, line 376 exercised) + """ + fc_parameters: dict[str, Any] = {"BATT_MONITOR": 5} # I2C type, no BATT_I2C_BUS + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning"): + basic_model._set_battery_type_from_fc_parameters(fc_parameters) + + result = basic_model.get_component_value(("Battery Monitor", "FC Connection", "Type")) + assert result == "I2C1" + + # ------------------------------------------------------------------ + # _set_serial_type_from_fc_parameters — GNSS CAN type prevents serial override (line 300->302) + # ------------------------------------------------------------------ + def test_system_skips_gnss_type_override_when_type_is_can(self, basic_model) -> None: + """ + _set_serial_type_from_fc_parameters skips setting GNSS type when already set to a CAN port. + + GIVEN: GNSS Receiver FC Connection Type is set to 'CAN1' (already configured via CAN) + AND: SERIAL3_PROTOCOL=5 (GPS protocol) is in fc_parameters + WHEN: _set_serial_type_from_fc_parameters is called + THEN: GNSS Receiver Type stays as 'CAN1' (not overwritten by SERIAL3) + AND: Line 300->302 (False branch of 'if current_gnss_type not in CAN_PORTS:') is exercised + """ + basic_model.set_component_value(("GNSS Receiver", "FC Connection", "Type"), "CAN1") + fc_parameters: dict[str, Any] = {"SERIAL3_PROTOCOL": 5} # GPS on SERIAL3 + + basic_model._set_serial_type_from_fc_parameters(fc_parameters) + + result = basic_model.get_component_value(("GNSS Receiver", "FC Connection", "Type")) + assert result == "CAN1" # Not overwritten + + # ------------------------------------------------------------------ + # _set_serial_type_from_fc_parameters — second RC Receiver falls through elif chain (line 303->263) + # ------------------------------------------------------------------ + def test_system_skips_second_rc_receiver_serial_assignment(self, basic_model) -> None: + """ + _set_serial_type_from_fc_parameters ignores second RC Receiver when one was already assigned. + + GIVEN: Two serial ports both configured with RCIN (RC Receiver) protocol=23 + WHEN: _set_serial_type_from_fc_parameters is called + THEN: Only the first RC Receiver is assigned; the second falls through all elif branches + AND: Line 303->263 (elif component == 'ESC' False branch) is exercised + """ + fc_parameters: dict[str, Any] = { + "SERIAL1_PROTOCOL": 23, # RCIN = RC Receiver, first → assigned + "SERIAL2_PROTOCOL": 23, # RCIN = RC Receiver, second → rc > 1 → skipped + } + + basic_model._set_serial_type_from_fc_parameters(fc_parameters) + + result = basic_model.get_component_value(("RC Receiver", "FC Connection", "Type")) + assert result == "SERIAL1" # Only first RC Receiver assigned diff --git a/tests/test_data_model_vehicle_components_json_schema.py b/tests/test_data_model_vehicle_components_json_schema.py index ac5c804e8..9a69cc751 100755 --- a/tests/test_data_model_vehicle_components_json_schema.py +++ b/tests/test_data_model_vehicle_components_json_schema.py @@ -14,6 +14,7 @@ from unittest.mock import patch import pytest +from test_data_model_vehicle_components_common import ComponentDataModelFixtures, make_fc_schema from ardupilot_methodic_configurator.data_model_vehicle_components_json_schema import VehicleComponentsJsonSchema @@ -481,3 +482,65 @@ def test_schema_initialization_and_attribute_access(self, minimal_schema) -> Non # Test that the schema reference is maintained assert json_schema.schema["properties"]["Components"] is not None assert "Flight Controller" in json_schema.schema["properties"]["Components"]["properties"] + + +class TestVehicleComponentsJsonSchemaUncoveredBranches: + """Tests targeting previously uncovered branches in VehicleComponentsJsonSchema.""" + + def test_system_extracts_datatypes_from_property_with_allof_construct(self) -> None: + """ + _extract_datatypes_from_property handles a property whose schema uses 'allOf'. + + GIVEN: A schema where a component property uses 'allOf' (not 'properties' or 'type' directly) + WHEN: get_all_value_datatypes is called + THEN: The allOf branch is taken and the types are extracted into the target dict + """ + schema_with_allof_property = { + "properties": { + "Components": {"properties": {"Frame": {"properties": {"Config": {"allOf": [{"type": "string"}]}}}}} + } + } + json_schema = VehicleComponentsJsonSchema(schema_with_allof_property) + + result = json_schema.get_all_value_datatypes() + + assert "Frame" in result + # The allOf branch creates an empty dict (target_dict[prop_name] = {}) + assert "Config" in result["Frame"] + + def test_system_returns_empty_description_for_component_not_in_schema(self) -> None: + """ + _get_nested_property_description returns ('', False) when component type is absent from schema. + + GIVEN: A real schema that contains 'Battery' but not 'UnknownComponent' + WHEN: get_component_property_description is called with path ('UnknownComponent', 'Spec', 'Field') + THEN: ('', False) should be returned without error + """ + real_schema = ComponentDataModelFixtures.create_schema() + + description, is_optional = real_schema.get_component_property_description( + ("UnknownComponent", "Specifications", "SomeField") + ) + + assert description == "" + assert is_optional is False + + def test_system_returns_empty_description_for_missing_section_with_len2_path(self) -> None: + """ + _get_section_field_description returns ('', False) when the section is not found in schema. + + GIVEN: A schema for 'Flight Controller' + WHEN: get_component_property_description is called with a 2-element path pointing to a + non-existent section + THEN: ('', False) should be returned + """ + schema = make_fc_schema( + {"allOf": [{"properties": {"Firmware": {"description": "Firmware info", "x-is-optional": False}}}]}, + definitions={}, + ) + json_schema = VehicleComponentsJsonSchema(schema) + + description, is_optional = json_schema.get_component_property_description(("Flight Controller", "NonExistentSection")) + + assert description == "" + assert is_optional is False diff --git a/tests/test_data_model_vehicle_components_templates.py b/tests/test_data_model_vehicle_components_templates.py index d56a95dc6..cff797117 100755 --- a/tests/test_data_model_vehicle_components_templates.py +++ b/tests/test_data_model_vehicle_components_templates.py @@ -1192,3 +1192,31 @@ def test_system_converts_mixed_numeric_strings(self, empty_model) -> None: assert result["SerialNumber"] == "SN-ABC123" # string assert result["CalibrationValue"] == 3.14159 # float assert result["Notes"] == "Calibrated on 2024-01-01" # string + + +class TestComponentDataModelTemplatesUncoveredBranches: # pylint: disable=too-few-public-methods + """Tests targeting previously uncovered branches in ComponentDataModelTemplates (line 30).""" + + def test_system_creates_components_key_when_data_has_no_components_key(self) -> None: + """ + update_component creates the 'Components' dict when _data lacks the key. + + GIVEN: A model whose _data does not contain a 'Components' key + WHEN: update_component is called + THEN: 'Components' should be created in _data + AND: The new component should be stored under it + """ + component_datatypes = ComponentDataModelFixtures.create_component_datatypes() + schema = ComponentDataModelFixtures.create_schema() + # {"Format version": 1} is truthy → __init__ uses deepcopy → _data has no "Components" + model = ComponentDataModelTemplates({"Format version": 1}, component_datatypes, schema) + # At this point _data = {"Format version": 1} with no "Components" key + # pylint: disable=protected-access + assert "Components" not in model._data + + model.update_component("NewComponent", {"Some": "data"}) + + assert "Components" in model._data + assert "NewComponent" in model._data["Components"] + assert model._data["Components"]["NewComponent"] == {"Some": "data"} + # pylint: enable=protected-access diff --git a/tests/test_data_model_vehicle_components_validation.py b/tests/test_data_model_vehicle_components_validation.py index d1870b3e1..4a68b6a67 100755 --- a/tests/test_data_model_vehicle_components_validation.py +++ b/tests/test_data_model_vehicle_components_validation.py @@ -11,6 +11,7 @@ """ import math +from unittest.mock import patch as _patch import pytest from test_data_model_vehicle_components_common import ( @@ -1527,3 +1528,482 @@ def test_low_voltage_validation_requires_below_arm_not_max(self, realistic_model # Assert assert err != "" assert corr is not None + + +class TestComponentDataModelValidationUncoveredBranches: + """Tests targeting previously uncovered branches in ComponentDataModelValidation.""" + + @pytest.fixture + def basic_model(self) -> ComponentDataModelValidation: + """Create a basic model.""" + return ComponentDataModelFixtures.create_basic_model(ComponentDataModelValidation) + + @pytest.fixture + def realistic_model(self) -> ComponentDataModelValidation: + """Create a realistic model with full battery and frame specs.""" + return ComponentDataModelFixtures.create_realistic_model(ComponentDataModelValidation) + + # ------------------------------------------------------------------ + # init_possible_choices - error logging paths (lines 309-310) + # ------------------------------------------------------------------ + def test_system_logs_error_when_q_m_pwm_type_entry_has_no_values(self, basic_model) -> None: + """ + init_possible_choices logs an error when Q_M_PWM_TYPE entry has no values or Bitmask. + + GIVEN: A doc_dict where Q_M_PWM_TYPE exists but lacks both 'values' and 'Bitmask' + WHEN: init_possible_choices is called + THEN: Errors are logged for missing values AND for missing fallback + """ + doc_dict = {"Q_M_PWM_TYPE": {}} # key present, no values/Bitmask; NOT in fallbacks + + with _patch("ardupilot_methodic_configurator.data_model_vehicle_components_validation.logging_error") as mock_err: + basic_model.init_possible_choices(doc_dict) + + error_calls = " ".join(str(c) for c in mock_err.call_args_list) + assert "Q_M_PWM_TYPE" in error_calls or "No values" in error_calls or "fallback" in error_calls + + # ------------------------------------------------------------------ + # _update_possible_choices_for_path - None connection type branches + # ------------------------------------------------------------------ + def test_system_restricts_rc_receiver_protocols_to_none_for_none_connection(self, basic_model) -> None: + """ + _update_possible_choices_for_path sets RC Receiver protocol to ('None',) when type=None. + + GIVEN: A model with initialized possible choices + WHEN: _update_possible_choices_for_path is called for RC Receiver with type 'None' + THEN: The protocol choices should be ('None',) + """ + basic_model.init_possible_choices({}) + + basic_model._update_possible_choices_for_path(("RC Receiver", "FC Connection", "Type"), "None") + + assert basic_model._possible_choices[("RC Receiver", "FC Connection", "Protocol")] == ("None",) + + def test_system_restricts_battery_monitor_protocols_to_none_for_none_connection(self, basic_model) -> None: + """ + _update_possible_choices_for_path restricts Battery Monitor protocols to ('None',) for None type. + + GIVEN: A model with initialized possible choices + WHEN: _update_possible_choices_for_path is called for Battery Monitor with type 'None' + THEN: Protocol choices should be ('None',) + """ + basic_model.init_possible_choices({}) + + basic_model._update_possible_choices_for_path(("Battery Monitor", "FC Connection", "Type"), "None") + + assert basic_model._possible_choices[("Battery Monitor", "FC Connection", "Protocol")] == ("None",) + + def test_system_restricts_esc_protocols_to_none_for_none_connection(self, basic_model) -> None: + """ + _update_possible_choices_for_path sets ESC protocol to ('None',) when type=None. + + GIVEN: A model with initialized possible choices + WHEN: _update_possible_choices_for_path is called for ESC with type 'None' + THEN: Protocol choices should be ('None',) + """ + basic_model.init_possible_choices({}) + + basic_model._update_possible_choices_for_path(("ESC", "FC Connection", "Type"), "None") + + assert basic_model._possible_choices[("ESC", "FC Connection", "Protocol")] == ("None",) + + def test_system_sets_esc_protocol_to_dronecan_for_can_connection(self, basic_model) -> None: + """ + _update_possible_choices_for_path sets ESC protocol to ('DroneCAN',) for a CAN port. + + GIVEN: A model with initialized possible choices + WHEN: _update_possible_choices_for_path is called for ESC with type 'CAN1' + THEN: Protocol choices should be ('DroneCAN',) + """ + basic_model.init_possible_choices({}) + + basic_model._update_possible_choices_for_path(("ESC", "FC Connection", "Type"), "CAN1") + + assert basic_model._possible_choices[("ESC", "FC Connection", "Protocol")] == ("DroneCAN",) + + def test_system_sets_esc_protocols_for_serial_connection(self, basic_model) -> None: + """ + _update_possible_choices_for_path populates serial ESC protocol choices for SERIAL1. + + GIVEN: A model with initialized possible choices + WHEN: _update_possible_choices_for_path is called for ESC with type 'SERIAL1' + THEN: Protocol choices should be a non-empty tuple of serial ESC protocols + """ + basic_model.init_possible_choices({}) + + basic_model._update_possible_choices_for_path(("ESC", "FC Connection", "Type"), "SERIAL1") + + choices = basic_model._possible_choices[("ESC", "FC Connection", "Protocol")] + assert isinstance(choices, tuple) + assert len(choices) > 0 + + # ------------------------------------------------------------------ + # validate_entry_limits - TOW max < TOW min (line 514) + # ------------------------------------------------------------------ + def test_system_rejects_tow_max_below_tow_min(self, realistic_model) -> None: + """ + validate_entry_limits returns an error when TOW max is lower than the existing TOW min. + + GIVEN: A model where TOW min = 0.6 kg (realistic data) + WHEN: validate_entry_limits is called with TOW max = 0.5 kg + THEN: An error message should be returned + """ + error_msg, corrected = realistic_model.validate_entry_limits("0.5", ("Frame", "Specifications", "TOW max Kg")) + + assert error_msg != "" + assert corrected is not None + + # ------------------------------------------------------------------ + # validate_cell_voltage - ValueError and cross-voltage branches + # ------------------------------------------------------------------ + def test_system_handles_non_numeric_cell_voltage_with_recommended_correction(self, realistic_model) -> None: + """ + validate_cell_voltage returns an error and the recommended voltage for non-numeric input. + + GIVEN: A string that cannot be converted to float + WHEN: validate_cell_voltage is called for Volt per cell max + THEN: An error message and the recommended cell voltage should be returned + """ + err_msg, corrected = realistic_model.validate_cell_voltage( + "not_a_number", ("Battery", "Specifications", "Volt per cell max") + ) + + assert err_msg != "" + assert isinstance(corrected, float) + + def test_system_rejects_max_voltage_below_arm_voltage(self, realistic_model) -> None: + """ + validate_cell_voltage returns an error when Volt per cell max < Volt per cell arm. + + GIVEN: Volt per cell arm = 3.85 already stored + WHEN: validate_cell_voltage is called with max = 3.7 (below arm) + THEN: An error should be returned + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell arm"), 3.85) + + err_msg, _corrected = realistic_model.validate_cell_voltage("3.7", ("Battery", "Specifications", "Volt per cell max")) + + assert err_msg != "" + + def test_system_accepts_max_voltage_above_arm_voltage(self, realistic_model) -> None: + """ + validate_cell_voltage returns no error when Volt per cell max > arm. + + GIVEN: Volt per cell arm = 3.85 + WHEN: validate_cell_voltage is called with max = 4.2 + THEN: No error should be returned + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell arm"), 3.85) + + err_msg, _corrected = realistic_model.validate_cell_voltage("4.2", ("Battery", "Specifications", "Volt per cell max")) + + assert err_msg == "" + + def test_system_rejects_arm_voltage_above_max_voltage(self, realistic_model) -> None: + """ + validate_cell_voltage rejects arm voltage when it exceeds max. + + GIVEN: Volt per cell max = 4.2 + WHEN: validate_cell_voltage is called with arm = 4.3 + THEN: An error should be returned + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell max"), 4.2) + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell low"), 3.6) + + err_msg, _corrected = realistic_model.validate_cell_voltage("4.3", ("Battery", "Specifications", "Volt per cell arm")) + + assert err_msg != "" + + def test_system_accepts_arm_voltage_between_max_and_low(self, realistic_model) -> None: + """ + validate_cell_voltage passes when arm is between max and low. + + GIVEN: max=4.2, low=3.6 + WHEN: validate_cell_voltage is called with arm = 3.85 + THEN: No error should be returned + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell max"), 4.2) + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell low"), 3.6) + + err_msg, _corrected = realistic_model.validate_cell_voltage("3.85", ("Battery", "Specifications", "Volt per cell arm")) + + assert err_msg == "" + + def test_system_rejects_crit_voltage_above_low_voltage(self, realistic_model) -> None: + """ + validate_cell_voltage returns an error when Volt per cell crit exceeds low. + + GIVEN: low = 3.6 + WHEN: validate_cell_voltage is called with crit = 3.7 (above low) + THEN: An error should be returned + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell low"), 3.6) + + err_msg, _corrected = realistic_model.validate_cell_voltage("3.7", ("Battery", "Specifications", "Volt per cell crit")) + + assert err_msg != "" + + def test_system_accepts_crit_voltage_below_low_voltage(self, realistic_model) -> None: + """ + validate_cell_voltage passes when Volt per cell crit is below low. + + GIVEN: low = 3.6 + WHEN: validate_cell_voltage is called with crit = 3.5 + THEN: No error should be returned + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell low"), 3.6) + + err_msg, _corrected = realistic_model.validate_cell_voltage("3.5", ("Battery", "Specifications", "Volt per cell crit")) + + assert err_msg == "" + + def test_system_rejects_min_voltage_above_low_voltage(self, realistic_model) -> None: + """ + validate_cell_voltage returns an error when Volt per cell min exceeds low. + + GIVEN: low = 3.6 + WHEN: validate_cell_voltage is called with min = 3.7 + THEN: An error should be returned + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell low"), 3.6) + + err_msg, _corrected = realistic_model.validate_cell_voltage("3.7", ("Battery", "Specifications", "Volt per cell min")) + + assert err_msg != "" + + def test_system_accepts_min_voltage_below_low_voltage(self, realistic_model) -> None: + """ + validate_cell_voltage passes when Volt per cell min is below low. + + GIVEN: low = 3.6 + WHEN: validate_cell_voltage is called with min = 3.0 + THEN: No error should be returned + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell low"), 3.6) + + err_msg, _corrected = realistic_model.validate_cell_voltage("3.0", ("Battery", "Specifications", "Volt per cell min")) + + assert err_msg == "" + + # ------------------------------------------------------------------ + # validate_cell_voltage — arm > stored max cross-validation early return (line 514 area) + # ------------------------------------------------------------------ + def test_system_rejects_arm_voltage_above_stored_max_within_chemistry_limits(self, realistic_model) -> None: + """ + validate_cell_voltage returns an early error when arm voltage exceeds the stored Volt per cell max. + + GIVEN: Volt per cell max is set to 3.9 (within Lipo limits 3.0-4.2) + AND: Volt per cell arm = 4.1 (also within Lipo limits, but above max) + WHEN: validate_cell_voltage is called for arm + THEN: An error is returned immediately from the cross-validation against max + AND: The early-return branch (if err_msg: return err_msg, corr) is exercised + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell max"), 3.9) + + err_msg, corr = realistic_model.validate_cell_voltage("4.1", ("Battery", "Specifications", "Volt per cell arm")) + + assert err_msg != "" + assert corr is not None # corrected value is returned + + # ------------------------------------------------------------------ + # validate_cell_voltage — unknown "Volt per cell X" type falls through to return "", None + # ------------------------------------------------------------------ + def test_system_accepts_unknown_volt_per_cell_type_without_cross_validation(self, basic_model) -> None: + """ + validate_cell_voltage returns ("", None) for unknown Volt per cell type names. + + GIVEN: A path with "Volt per cell" in path[2] but not matching any of the 5 known types + WHEN: validate_cell_voltage is called directly with a valid voltage + THEN: No error is returned (the fall-through 'return "", None' is exercised) + """ + # "Volt per cell custom" contains "Volt per cell" → passes outer if + # But is not max/arm/low/crit/min → all 5 inner if-checks are False → fall-through + err_msg, corr = basic_model.validate_cell_voltage("3.7", ("Battery", "Specifications", "Volt per cell custom")) + + assert err_msg == "" + assert corr is None + + # ------------------------------------------------------------------ + # validate_all_data — Telemetry+RC sharing a serial port → allowed (line 614 area) + # ------------------------------------------------------------------ + def test_system_allows_telemetry_and_rc_receiver_to_share_serial_port(self, basic_model) -> None: + """ + validate_all_data does NOT report an error when Telemetry and RC Receiver share a serial port. + + GIVEN: Both Telemetry and RC Receiver FC Connection Types are set to 'SERIAL1' + WHEN: validate_all_data is called + THEN: No duplicate connection error is raised for this combination (continue is executed) + AND: The 'if path[0] in Telemetry/RC Receiver' branch (line ~614) is exercised + """ + entry_values = { + ("Telemetry", "FC Connection", "Type"): "SERIAL1", + ("RC Receiver", "FC Connection", "Type"): "SERIAL1", # duplicate → allowed for Telemetry+RC + } + + _is_valid, errors = basic_model.validate_all_data(entry_values) + + # Verify no duplicate connection errors are raised for this pair + duplicate_errors = [e for e in errors if "Duplicate FC connection type" in e] + assert len(duplicate_errors) == 0 + + # ------------------------------------------------------------------ + # validate_all_data — VALIDATION_RULES returns non-None corrected_value (line 637 area) + # ------------------------------------------------------------------ + def test_system_corrects_and_stores_out_of_range_value_via_validation_rules(self, basic_model) -> None: + """ + validate_all_data stores the corrected value when VALIDATION_RULES rejects with a non-None correction. + + GIVEN: TOW max Kg is set to 700 (above max allowed 600) + WHEN: validate_all_data is called + THEN: An error is reported for the out-of-range value + AND: set_component_value is called with the corrected (clamped) value 600.0 + AND: The 'if corrected_value is not None: set_component_value' branch is exercised + """ + # "700" exceeds the (0.01, 600) range in VALIDATION_RULES → returns error + corrected=600.0 + entry_values = {("Frame", "Specifications", "TOW max Kg"): "700"} + + is_valid, errors = basic_model.validate_all_data(entry_values) + + assert not is_valid + assert len(errors) > 0 + # Verify the model was updated with the corrected value + corrected = basic_model.get_component_value(("Frame", "Specifications", "TOW max Kg")) + assert corrected == 600.0 + + # ------------------------------------------------------------------ + # validate_entry_limits — TOW max with lim=0 (if lim: False branch — lines 464/473 area) + # ------------------------------------------------------------------ + def test_system_skips_tow_cross_validation_when_tow_min_is_zero(self, basic_model) -> None: + """ + validate_entry_limits skips TOW max cross-check when TOW min is zero (falsy). + + GIVEN: TOW min Kg is explicitly set to 0 + AND: TOW max Kg value is provided for validation + WHEN: validate_entry_limits is called for TOW max Kg + THEN: No cross-validation error is returned (lim is falsy → if lim: is False) + AND: The False branch of 'if lim: return validate_against_another_value(...)' is exercised + """ + basic_model.set_component_value(("Frame", "Specifications", "TOW min Kg"), 0) + + err_msg, corr = basic_model.validate_entry_limits("1.0", ("Frame", "Specifications", "TOW max Kg")) + + assert err_msg == "" + assert corr is None + + def test_system_skips_tow_cross_validation_when_tow_max_is_zero(self, basic_model) -> None: + """ + validate_entry_limits skips TOW min cross-check when TOW max is zero (falsy). + + GIVEN: TOW max Kg is explicitly set to 0 + AND: TOW min Kg value is provided for validation + WHEN: validate_entry_limits is called for TOW min Kg + THEN: No cross-validation error is returned (lim is falsy → if lim: is False) + AND: The False branch of 'if lim: return validate_against_another_value(...)' is exercised + """ + basic_model.set_component_value(("Frame", "Specifications", "TOW max Kg"), 0) + + err_msg, corr = basic_model.validate_entry_limits("1.0", ("Frame", "Specifications", "TOW min Kg")) + + assert err_msg == "" + assert corr is None + + # ------------------------------------------------------------------ + # _validate_motor_poles — odd poles (existing coverage) and even poles (verify) + # ------------------------------------------------------------------ + def test_system_reports_error_for_odd_motor_poles_via_validate_all_data(self, realistic_model) -> None: + """ + validate_all_data reports an error when motor poles count is odd. + + GIVEN: Motor Poles is set to 13 (odd, within 2-59 range, passes VALIDATION_RULES int check) + WHEN: validate_all_data is called + THEN: An error is returned about needing even poles + AND: _validate_motor_poles (lines 629-636) is exercised + """ + entry_values = {("Motors", "Specifications", "Poles"): "13"} + + is_valid, errors = realistic_model.validate_all_data(entry_values) + + assert not is_valid + assert any("even" in e for e in errors) + + # ------------------------------------------------------------------ + # validate_all_data — Battery Monitor + ESC sharing serial port allowed (line 598 area) + # ------------------------------------------------------------------ + def test_system_reports_duplicate_error_when_esc_and_telemetry_share_serial_port(self, basic_model) -> None: + """ + validate_all_data reports a duplicate error when ESC and Telemetry share a serial port. + + GIVEN: Both Telemetry and ESC FC Connection Type = 'SERIAL1' + WHEN: validate_all_data is called + THEN: A duplicate connection error is raised (ESC+Telemetry is not an exempt combination) + """ + entry_values = { + ("Telemetry", "FC Connection", "Type"): "SERIAL1", + ("ESC", "FC Connection", "Type"): "SERIAL1", + } + + _is_valid, errors = basic_model.validate_all_data(entry_values) + + duplicate_errors = [e for e in errors if "Duplicate FC connection type" in e] + assert len(duplicate_errors) >= 1 + + # ------------------------------------------------------------------ + # validate_all_data — battery cell voltage error corrected and stored (lines 616-621 area) + # ------------------------------------------------------------------ + def test_system_corrects_and_stores_out_of_range_cell_voltage_via_validate_all_data(self, realistic_model) -> None: + """ + validate_all_data corrects and stores an out-of-range battery cell voltage. + + GIVEN: 'Volt per cell max' is set to 4.5 (above Lipo chemistry limit of 4.2) + WHEN: validate_all_data is called + THEN: An error is reported for the out-of-range voltage + AND: set_component_value is called with the corrected limit (4.2) + AND: Lines 616-621 (validate_cell_voltage error + corrected_value handling) are exercised + """ + entry_values = {("Battery", "Specifications", "Volt per cell max"): "4.5"} + + is_valid, errors = realistic_model.validate_all_data(entry_values) + + assert not is_valid + assert len(errors) > 0 + # Verify the model was updated with the corrected (clamped) value + corrected = realistic_model.get_component_value(("Battery", "Specifications", "Volt per cell max")) + assert corrected == 4.2 # Lipo limit_max + + def test_system_does_not_error_for_valid_cell_voltage_via_validate_all_data(self, realistic_model) -> None: + """ + validate_all_data doesn't add an error for a valid Volt per cell crit value. + + GIVEN: 'Volt per cell crit' is 3.5 (below Volt per cell low = 3.6, within Lipo limits) + WHEN: validate_all_data is called with a valid value + THEN: No error is raised for this entry + AND: The 'if error_msg: → False' branch (614->620 area) in the cell voltage block is exercised + """ + realistic_model.set_component_value(("Battery", "Specifications", "Volt per cell low"), 3.6) + entry_values = {("Battery", "Specifications", "Volt per cell crit"): "3.5"} + + is_valid, errors = realistic_model.validate_all_data(entry_values) + + cell_errors = [e for e in errors if "Volt per cell crit" in e or "crit" in e.lower()] + assert is_valid + assert len(cell_errors) == 0 + + def test_system_reports_error_without_correction_when_arm_limit_is_not_set(self, basic_model) -> None: + """ + validate_all_data reports a cell voltage cross-validation error but with None corrected_value. + + GIVEN: 'Volt per cell arm' is not set in the model (returns None for the limit) + AND: 'Volt per cell max' = 4.1 is passed to validate_all_data + WHEN: validate_all_data is called + THEN: An error is returned from validate_against_another_value (limit not a number) + AND: corrected_value is None → 'if corrected_value is not None: → False' (616->618 area) + """ + # basic_model's Battery has no Volt per cell arm set → get_component_value returns None + # → validate_against_another_value returns (error_msg, None) with None corrected + entry_values = {("Battery", "Specifications", "Volt per cell max"): "4.1"} + + is_valid, errors = basic_model.validate_all_data(entry_values) + + # Error should be returned about arm not being set + assert not is_valid + assert len(errors) > 0 diff --git a/tests/unit_battery_cell_voltages.py b/tests/unit_battery_cell_voltages.py new file mode 100755 index 000000000..2e472c5f2 --- /dev/null +++ b/tests/unit_battery_cell_voltages.py @@ -0,0 +1,107 @@ +#!/usr/bin/env python3 + +""" +Unit tests for the battery_cell_voltages.py file. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +from math import isnan + +import pytest + +from ardupilot_methodic_configurator.battery_cell_voltages import BatteryCell, _recommended_battery_cell_voltages + + +class TestBatteryCell: + """Unit tests for the BatteryCell class.""" + + def test_chemistries(self) -> None: + expected_chemistries = ("LiIon", "LiIonSS", "LiIonSSHV", "Lipo", "LipoHV", "LipoHVSS", "NiCd", "NiMH") + chemistries = BatteryCell.chemistries() + assert chemistries == expected_chemistries + + def test_limit_max_voltage(self) -> None: + assert BatteryCell.limit_max_voltage("LiIon") == 4.2 + assert BatteryCell.limit_max_voltage("LipoHV") == 4.35 + assert BatteryCell.limit_max_voltage("NonExistentChemistry") == 4.45 + + def test_limit_min_voltage(self) -> None: + assert BatteryCell.limit_min_voltage("LiIon") == 2.5 + assert BatteryCell.limit_min_voltage("LipoHV") == 3.0 + assert BatteryCell.limit_min_voltage("NonExistentChemistry") == 1.0 + + def test_recommended_max_voltage(self) -> None: + assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell max") == 4.1 + assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell max")) + + def test_recommended_low_voltage(self) -> None: + assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell low") == 3.1 + assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell low")) + + def test_recommended_crit_voltage(self) -> None: + assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell crit") == 2.8 + assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell crit")) + + def test_recommended_arm_voltage(self) -> None: + assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell arm") == 3.6 + assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell arm")) + + def test_recommended_min_voltage(self) -> None: + assert BatteryCell.recommended_cell_voltage("LiIon", "Volt per cell min") == 2.7 + assert isnan(BatteryCell.recommended_cell_voltage("NonExistentChemistry", "Volt per cell min")) + + @pytest.mark.parametrize("chem", BatteryCell.chemistries()) + def test_voltage_monotonicity(self, chem: str) -> None: + assert BatteryCell.limit_max_voltage(chem) == _recommended_battery_cell_voltages[chem].get("absolute_max") + assert BatteryCell.limit_min_voltage(chem) == _recommended_battery_cell_voltages[chem].get("absolute_min") + assert BatteryCell.limit_max_voltage(chem) >= BatteryCell.recommended_cell_voltage(chem, "Volt per cell max") + assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell max") >= BatteryCell.recommended_cell_voltage( + chem, "Volt per cell arm" + ) + assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell arm") >= BatteryCell.recommended_cell_voltage( + chem, "Volt per cell low" + ) + assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell low") >= BatteryCell.recommended_cell_voltage( + chem, "Volt per cell crit" + ) + # This is not required, the user might want to stop PID scaling above the critical voltage + # assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell crit") >= + # BatteryCell.recommended_cell_voltage(chem, "Volt per cell min") + assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell min") >= BatteryCell.limit_min_voltage(chem) + assert BatteryCell.recommended_cell_voltage(chem, "Volt per cell crit") >= BatteryCell.limit_min_voltage(chem) + + def test_chemistry_voltage_score_returns_nan_for_non_positive_total_voltage(self) -> None: + """chemistry_voltage_score returns nan when total_voltage is zero or negative.""" + # total_voltage <= 0 → early return nan (covers the branch at the very top of the function) + assert isnan(BatteryCell.chemistry_voltage_score("Lipo", 0.0, "Volt per cell max")) + assert isnan(BatteryCell.chemistry_voltage_score("Lipo", -1.0, "Volt per cell max")) + + def test_chemistry_voltage_score_returns_nan_for_unknown_chemistry(self) -> None: + """chemistry_voltage_score returns nan when chemistry has no recommended volt-per-cell.""" + # recommended_cell_voltage returns nan for unknown chemistry → inner nan branch + assert isnan(BatteryCell.chemistry_voltage_score("NonExistentChem", 16.8, "Volt per cell max")) + + def test_chemistry_voltage_score_returns_float_for_valid_inputs(self) -> None: + """chemistry_voltage_score returns a non-nan float for valid chemistry, voltage and type.""" + score = BatteryCell.chemistry_voltage_score("Lipo", 16.8, "Volt per cell max") + assert not isnan(score) + assert 0.0 <= score < 0.5 # 16.8 / 4.2 = 4.0 cells → score should be 0.0 + + def test_best_chemistry_for_voltage_returns_none_or_valid_chemistry_for_ambiguous_voltage(self) -> None: + """best_chemistry_for_voltage may return None or a known chemistry for an ambiguous voltage.""" + # 10.0 V may be ambiguous depending on scoring; if a match is returned, it must be a known chemistry. + result = BatteryCell.best_chemistry_for_voltage(10.0, "Volt per cell max") + assert result is None or result in BatteryCell.chemistries() + + def test_best_chemistry_for_voltage_identifies_chemistry_for_4s_voltage(self) -> None: + """best_chemistry_for_voltage returns a chemistry for a classic 4S full-charge voltage.""" + result = BatteryCell.best_chemistry_for_voltage(16.8, "Volt per cell max") + # 16.8 / 4.2 = exactly 4.0 cells → a clear winner should be found + assert result is not None + assert isinstance(result, str) + assert result in BatteryCell.chemistries() diff --git a/tests/unit_data_model_vehicle_components_base.py b/tests/unit_data_model_vehicle_components_base.py index 909e069e3..c06532539 100755 --- a/tests/unit_data_model_vehicle_components_base.py +++ b/tests/unit_data_model_vehicle_components_base.py @@ -470,3 +470,169 @@ def test_data_structure_validation(self) -> None: valid_data = {"Components": {"Battery": {}}, "Format version": 1} model = ComponentDataModelBase(valid_data, {}, schema) assert "Components" in model._data + + +class TestComponentDataModelBaseUncoveredBranches: + """Tests targeting previously uncovered branches in ComponentDataModelBase.""" + + @pytest.fixture + def basic_model(self) -> ComponentDataModelBase: + """Create a basic ComponentDataModelBase fixture for testing.""" + return ComponentDataModelFixtures.create_basic_model(ComponentDataModelBase) + + @pytest.fixture + def realistic_model(self) -> ComponentDataModelBase: + """Create a realistic ComponentDataModelBase fixture for testing.""" + return ComponentDataModelFixtures.create_realistic_model(ComponentDataModelBase) + + def test_system_converts_list_value_to_string_in_get_component_value(self, basic_model) -> None: + """ + System converts non-primitive component values to strings. + + GIVEN: A component path whose stored value is a list (not str/int/float/dict) + WHEN: get_component_value is called + THEN: The return value should be the str() representation of the list + """ + # Inject a list directly bypassing the normal setter + basic_model._data["Components"]["Battery"]["Specifications"]["SomeList"] = [1, 2, 3] + + result = basic_model.get_component_value(("Battery", "Specifications", "SomeList")) + + assert isinstance(result, str) + assert result == str([1, 2, 3]) + + def test_system_handles_broken_datatypes_in_get_component_datatype(self, basic_model) -> None: + """ + System returns None when _component_datatypes structure raises an exception. + + GIVEN: A model whose _component_datatypes is a truthy non-dict (raises AttributeError on .get()) + WHEN: _get_component_datatype is called for any path + THEN: The except clause fires and the method gracefully returns None without raising + """ + # Use a truthy non-dict so the early `if not self._component_datatypes` guard passes, + # but list.get() then raises AttributeError, hitting the except block at lines 121-122. + basic_model._component_datatypes = [1, 2, 3] # type: ignore[assignment] + + result = basic_model._get_component_datatype(("Battery", "Specifications", "Chemistry")) + assert result is None + + def test_system_casts_none_to_empty_list_for_list_datatype(self, basic_model) -> None: + """ + System returns an empty list when casting None to list datatype. + + GIVEN: A value of None and a target datatype of list + WHEN: _safe_cast_value is called + THEN: An empty list should be returned + """ + result = basic_model._safe_cast_value(None, list, ("Battery", "Specifications", "Tags")) + assert result == [] + + def test_system_casts_none_to_empty_dict_for_dict_datatype(self, basic_model) -> None: + """ + System returns an empty dict when casting None to dict datatype. + + GIVEN: A value of None and a target datatype of dict + WHEN: _safe_cast_value is called + THEN: An empty dict should be returned + """ + result = basic_model._safe_cast_value(None, dict, ("Battery", "Specifications", "Config")) + assert result == {} + + def test_system_migrates_legacy_gnss_sbf_protocol_to_new_name(self, basic_model) -> None: + """ + System migrates legacy GNSS protocol names to their canonical equivalents. + + GIVEN: A model with a GNSS receiver using the legacy 'SBF' protocol name + WHEN: update_json_structure() is called + THEN: The protocol should be updated to 'Septentrio(SBF)' + """ + basic_model._data["Components"]["GNSS Receiver"] = {"FC Connection": {"Protocol": "SBF"}} + + basic_model.update_json_structure() + + assert basic_model._data["Components"]["GNSS Receiver"]["FC Connection"]["Protocol"] == "Septentrio(SBF)" + + def test_system_defaults_to_lipo_when_battery_chemistry_is_invalid(self, basic_model) -> None: + """ + System logs an error and falls back to Lipo when battery chemistry is unrecognised. + + GIVEN: A model where Battery.Specifications.Chemistry is set to an invalid value + WHEN: init_battery_chemistry() is called + THEN: _battery_chemistry should be set to 'Lipo' (the default) + AND: The data should also be corrected to 'Lipo' + """ + basic_model._data["Components"]["Battery"]["Specifications"]["Chemistry"] = "InvalidChem" + + basic_model.init_battery_chemistry() + + assert basic_model._battery_chemistry == "Lipo" + assert basic_model._data["Components"]["Battery"]["Specifications"]["Chemistry"] == "Lipo" + + def test_system_casts_string_yes_to_bool_true(self, basic_model) -> None: + """ + System converts string 'yes' to True when casting to bool datatype. + + GIVEN: A string value 'yes' and target datatype bool + WHEN: _safe_cast_value is called + THEN: True should be returned because 'yes' is in the truthy string set + """ + result = basic_model._safe_cast_value("yes", bool, ("Battery", "Specifications", "SomeFlag")) + + assert result is True + + def test_system_casts_integer_to_bool(self, basic_model) -> None: + """ + System converts a non-string, non-bool value to bool when casting to bool datatype. + + GIVEN: An integer value 1 and target datatype bool + WHEN: _safe_cast_value is called + THEN: True should be returned via bool(1) + """ + result = basic_model._safe_cast_value(1, bool, ("Battery", "Specifications", "SomeFlag")) + + assert result is True + + def test_system_migrates_battery_monitor_other_protocol_type(self, basic_model) -> None: + """ + System sets Battery Monitor FC Connection Type to 'other' for protocols that use other ports. + + GIVEN: A model with Battery Monitor using protocol 'ESC' (an OTHER_PORTS protocol) + WHEN: update_json_structure() is called + THEN: The FC Connection Type should be set to 'other' + """ + basic_model._data["Components"]["Battery Monitor"] = {"FC Connection": {"Protocol": "ESC"}} + + basic_model.update_json_structure() + + assert basic_model._data["Components"]["Battery Monitor"]["FC Connection"]["Type"] == "other" + + def test_system_skips_reordering_fields_absent_from_product_dict(self, basic_model) -> None: + """ + System correctly reorders product fields, skipping those absent from the original dict. + + GIVEN: A Battery Product dict that has 'Version' and 'URL' but no 'Manufacturer' or 'Model' + WHEN: update_json_structure() is called (which triggers _reorder_components) + THEN: Only 'Version' and 'URL' appear in the reordered product + AND: The 'if field in product:' False branch is exercised for missing fields + """ + basic_model._data["Components"]["Battery"]["Product"] = {"Version": "v1", "URL": "http://example.com"} + + basic_model.update_json_structure() + + product = basic_model._data["Components"]["Battery"]["Product"] + assert "Version" in product + assert "URL" in product + assert "Manufacturer" not in product + assert "Model" not in product + + def test_system_accepts_valid_battery_chemistry_without_changes(self, realistic_model) -> None: + """ + System keeps existing valid chemistry unchanged when init_battery_chemistry is called. + + GIVEN: A model whose Battery chemistry is already set to the valid value 'Lipo' + WHEN: init_battery_chemistry() is called + THEN: _battery_chemistry remains 'Lipo' and no error is logged (fall-through branch covered) + """ + realistic_model.init_battery_chemistry() + + assert realistic_model._battery_chemistry == "Lipo" diff --git a/tests/unit_data_model_vehicle_components_display.py b/tests/unit_data_model_vehicle_components_display.py new file mode 100755 index 000000000..e825e29ce --- /dev/null +++ b/tests/unit_data_model_vehicle_components_display.py @@ -0,0 +1,178 @@ +#!/usr/bin/env python3 + +""" +Unit tests for low-level ComponentDataModelDisplay implementation details. + +These tests verify internal methods and implementation details for coverage purposes. +For behavior-driven tests, see test_data_model_vehicle_components_display.py + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +from unittest.mock import MagicMock + +import pytest +from test_data_model_vehicle_components_common import ComponentDataModelFixtures + +from ardupilot_methodic_configurator.data_model_vehicle_components_display import ComponentDataModelDisplay + +# pylint: disable=redefined-outer-name + + +@pytest.fixture +def mock_schema() -> MagicMock: + """Fixture providing a mock schema for display testing.""" + return ComponentDataModelFixtures.create_mock_schema() + + +class TestDisplayUncoveredBranches: + """Tests targeting previously uncovered branches in ComponentDataModelDisplay.""" + + @pytest.fixture + def display_model(self, mock_schema) -> ComponentDataModelDisplay: + """Fixture providing a ComponentDataModelDisplay instance.""" + return ComponentDataModelFixtures.create_display_model_with_mock_schema(mock_schema) + + # ------------------------------------------------------------------ + # should_display_in_simple_mode - non-top-level dict with all-optional + # ------------------------------------------------------------------ + def test_system_hides_non_toplevel_dict_when_all_subfields_are_optional(self, display_model) -> None: + """ + should_display_in_simple_mode returns False for a non-top-level dict whose leaf fields are all marked optional. + + GIVEN: A call where path is non-empty (not top-level) and the value + is a dict with leaf fields that are all optional + WHEN: should_display_in_simple_mode is called in 'simple' mode + THEN: False should be returned + """ + # All fields optional → schema always returns (description, True) + display_model.schema.get_component_property_description.return_value = ("desc", True) + + value = {"Field1": "value1", "Field2": "value2"} + + result = display_model.should_display_in_simple_mode("Specifications", value, ["Flight Controller"], "simple") + + assert result is False + + def test_system_shows_non_toplevel_dict_when_at_least_one_field_is_required(self, display_model) -> None: + """ + should_display_in_simple_mode returns True for a non-top-level dict that has at least one non-optional leaf. + + GIVEN: A non-top-level dict where one leaf field is NOT optional + WHEN: should_display_in_simple_mode is called in 'simple' mode + THEN: True should be returned + """ + # First call returns optional=False (required field found) + display_model.schema.get_component_property_description.return_value = ("desc", False) + + value = {"RequiredField": "value1"} + + result = display_model.should_display_in_simple_mode("Specifications", value, ["Flight Controller"], "simple") + + assert result is True + + def test_system_recurses_into_nested_dicts_in_non_toplevel_path(self, display_model) -> None: + """ + should_display_in_simple_mode recursively checks nested dicts for required fields. + + GIVEN: A non-top-level dict value that contains a nested dict, and the + nested dict has a required leaf field + WHEN: should_display_in_simple_mode is called in 'simple' mode + THEN: True should be returned because the nested dict has a required field + """ + # Make the nested leaf field required (not optional) + display_model.schema.get_component_property_description.return_value = ("desc", False) + + # value = {"NestedSection": {"LeafField": "value"}} — nested dict structure + nested_value = {"LeafField": "value"} + value = {"NestedSection": nested_value} + + result = display_model.should_display_in_simple_mode("FCConnection", value, ["RC Receiver"], "simple") + + assert result is True + + # ------------------------------------------------------------------ + # prepare_non_leaf_widget_config - optional field with description (line 122) + # ------------------------------------------------------------------ + def test_system_appends_optional_hint_to_non_leaf_description(self, display_model) -> None: + """ + prepare_non_leaf_widget_config appends the optional hint to the description for optional keys. + + GIVEN: An optional component key with a non-empty description + WHEN: prepare_non_leaf_widget_config is called + THEN: The returned description should include the optional hint on a new line + """ + display_model.schema.get_component_property_description.return_value = ("Section info", True) + + config = display_model.prepare_non_leaf_widget_config("Product", {"Manufacturer": "Test"}, ["Flight Controller"]) + + assert "Section info" in config["description"] + assert "optional" in config["description"].lower() + assert "\n" in config["description"] + assert "blank" in config["description"].lower() + + # ------------------------------------------------------------------ + # should_display_in_simple_mode - branch 57->54: top-level loop continues + # after recursive call returns False + # ------------------------------------------------------------------ + def test_system_continues_toplevel_loop_when_recursive_sub_section_returns_false(self, display_model) -> None: + """ + Top-level loop continues when a recursive sub-section call returns False. + + GIVEN: A top-level component with two dict sub-sections, all leaf fields optional + WHEN: should_display_in_simple_mode is called in 'simple' mode + THEN: The first False recursive result causes loop continuation (not break), + and the overall result is False + """ + # All leaf fields are optional → recursive calls return False + display_model.schema.get_component_property_description.return_value = ("desc", True) + display_model.get_all_components = MagicMock(return_value=["TopComp"]) + + # Two dict sub-sections: for loop must continue past first False recursive result + value = {"Section1": {"leaf1": "v1"}, "Section2": {"leaf2": "v2"}} + + result = display_model.should_display_in_simple_mode("TopComp", value, [], "simple") + + assert result is False + + # ------------------------------------------------------------------ + # should_display_in_simple_mode - branch 70->82: non-top-level value is not a dict + # ------------------------------------------------------------------ + def test_system_returns_false_when_non_toplevel_value_is_not_a_dict(self, display_model) -> None: + """ + Non-top-level call returns False immediately when value is not a dict. + + GIVEN: A call with a non-empty path and a plain string as the value (not a dict) + WHEN: should_display_in_simple_mode is called in 'simple' mode + THEN: isinstance check fails, skipping the for-loop, and False is returned + """ + result = display_model.should_display_in_simple_mode("leaf_key", "a_string_value", ["parent"], "simple") + + assert result is False + + # ------------------------------------------------------------------ + # should_display_in_simple_mode - branch 75->72: non-top-level loop continues + # after recursive sub-dict call returns False + # ------------------------------------------------------------------ + def test_system_continues_non_toplevel_loop_when_nested_recursive_call_returns_false(self, display_model) -> None: + """ + Non-top-level loop continues when a nested recursive dict call returns False. + + GIVEN: A non-top-level component with two nested dict sub-sections, all leaves optional + WHEN: should_display_in_simple_mode is called in 'simple' mode + THEN: The first False recursive result causes loop continuation (not early return), + and the overall result is False + """ + # All leaf fields are optional → recursive calls return False + display_model.schema.get_component_property_description.return_value = ("desc", True) + + # Two dict sub-sections in a non-top-level path: loop must continue past first False + value = {"SubSection1": {"leaf1": "v1"}, "SubSection2": {"leaf2": "v2"}} + + result = display_model.should_display_in_simple_mode("Connection", value, ["Flight Controller"], "simple") + + assert result is False diff --git a/tests/unit_data_model_vehicle_components_import.py b/tests/unit_data_model_vehicle_components_import.py index 0c1be6f32..06721ed69 100755 --- a/tests/unit_data_model_vehicle_components_import.py +++ b/tests/unit_data_model_vehicle_components_import.py @@ -20,7 +20,9 @@ import pytest from test_data_model_vehicle_components_common import ComponentDataModelFixtures +from ardupilot_methodic_configurator.battery_cell_voltages import BATTERY_DEFAULT_CHEMISTRY, BatteryCell from ardupilot_methodic_configurator.data_model_vehicle_components_import import ComponentDataModelImport +from ardupilot_methodic_configurator.data_model_vehicle_components_validation import ComponentDataModelValidation # pylint: disable=protected-access,too-many-public-methods @@ -504,3 +506,67 @@ def test_estimate_cell_count_validates_range(self, realistic_model) -> None: fc_parameters = {"MOT_BAT_VOLT_MAX": 300.0} # ~71 cells realistic_model._estimate_battery_cell_count(fc_parameters) assert realistic_model.get_component_value(("Battery", "Specifications", "Number of cells")) == initial_cells + + def test_set_battery_type_warns_when_batt_monitor_not_in_fc_parameters(self, realistic_model) -> None: + """ + System logs a warning when BATT_MONITOR is absent from FC parameters. + + GIVEN: FC parameters with no BATT_MONITOR key + WHEN: Setting battery type from FC parameters + THEN: A warning should be logged about missing BATT_MONITOR + AND: The rest of the battery processing (chemistry, cells) still runs without crash + """ + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning") as mock_warn: + realistic_model._set_battery_type_from_fc_parameters({}) + mock_warn.assert_called() + + def test_set_battery_type_uses_default_chemistry_when_detection_returns_invalid_name(self, realistic_model) -> None: + """ + System falls back to BATTERY_DEFAULT_CHEMISTRY when detected chemistry is not a valid name. + + GIVEN: FC parameters where chemistry detection returns an unrecognised chemistry string + WHEN: Setting battery type from FC parameters + THEN: The invalid chemistry is replaced with BATTERY_DEFAULT_CHEMISTRY + AND: Subsequent voltage limit lookups use the default chemistry without error + """ + assert BATTERY_DEFAULT_CHEMISTRY in BatteryCell.chemistries() + + with ( + patch.object(realistic_model, "_detect_battery_chemistry_from_voltages", return_value="InvalidChemistry"), + patch("ardupilot_methodic_configurator.data_model_vehicle_components_import.logging_warning"), + ): + # Should not raise — invalid chemistry triggers fallback to default + realistic_model._set_battery_type_from_fc_parameters({"BATT_MONITOR": 4}) + + def test_estimate_battery_cell_count_returns_zero_when_cell_path_not_in_validation_rules(self, realistic_model) -> None: + """ + Battery cell count estimation returns 0 when the cell path is absent from VALIDATION_RULES. + + GIVEN: VALIDATION_RULES patched to be empty (no cell-count entry) + WHEN: Estimating battery cell count with a valid voltage parameter + THEN: Should return 0 (validation guard cannot be checked) + """ + realistic_model.set_component_value(("Battery", "Specifications", "Chemistry"), "Lipo") + + with patch.object(ComponentDataModelValidation, "VALIDATION_RULES", {}): + result = realistic_model._estimate_battery_cell_count({"MOT_BAT_VOLT_MAX": 16.8}) + + assert result == 0 + + def test_set_battery_type_handles_string_type_in_batt_monitor_connection(self, realistic_model) -> None: + """ + Battery type setter handles non-list (string) type in BATT_MONITOR_CONNECTION. + + GIVEN: BATT_MONITOR_CONNECTION patched to have a plain string 'type' (not a list) + WHEN: Setting battery type with that BATT_MONITOR value + THEN: isinstance(fc_conn_type, list) is False — the list-handling block is skipped + and the string type is set directly on the Battery Monitor component + """ + with patch( + "ardupilot_methodic_configurator.data_model_vehicle_components_import.BATT_MONITOR_CONNECTION", + {"9": {"type": "Other", "protocol": "ESC"}}, + ): + realistic_model._set_battery_type_from_fc_parameters({"BATT_MONITOR": 9}) + + batt_type = realistic_model.get_component_value(("Battery Monitor", "FC Connection", "Type")) + assert batt_type == "Other" diff --git a/tests/unit_data_model_vehicle_components_json_schema.py b/tests/unit_data_model_vehicle_components_json_schema.py index bb484f01c..a4f52ba19 100755 --- a/tests/unit_data_model_vehicle_components_json_schema.py +++ b/tests/unit_data_model_vehicle_components_json_schema.py @@ -17,6 +17,7 @@ from unittest.mock import patch import pytest +from test_data_model_vehicle_components_common import make_fc_schema from ardupilot_methodic_configurator.data_model_vehicle_components_json_schema import VehicleComponentsJsonSchema @@ -421,3 +422,276 @@ def test_error_handling_in_schema_modification(self) -> None: with patch("ardupilot_methodic_configurator.data_model_vehicle_components_json_schema.logging_error") as mock_log: json_schema.modify_schema_for_mcu_series(is_optional=True) mock_log.assert_called() + + def test_modify_schema_logs_error_when_flight_controller_def_has_no_allof(self) -> None: + """ + modify_schema_for_mcu_series logs an error when flightController definition has no allOf. + + GIVEN: A schema where definitions.flightController exists but contains no 'allOf' key + WHEN: modify_schema_for_mcu_series is called + THEN: The 'if allOf in flight_controller_def' check is False (arc 126->132), + flight_controller_properties stays None, and an error is logged + """ + schema_no_allof = { + "definitions": { + "flightController": { + "properties": {"Firmware": {"type": "string"}} # no 'allOf' key + } + } + } + json_schema = VehicleComponentsJsonSchema(schema_no_allof) + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_json_schema.logging_error") as mock_err: + json_schema.modify_schema_for_mcu_series(is_optional=True) + + mock_err.assert_called() + + def test_get_product_field_description_returns_empty_for_unknown_product_field(self) -> None: + """ + _get_product_field_description returns ('', False) when field_name is not in product definition. + + GIVEN: A schema with definitions.product containing 'Manufacturer' only + WHEN: get_component_property_description is called with a path like + ('FC', 'Product', 'NonExistentField') + THEN: ('', False) is returned (line 203 in _get_product_field_description) + """ + schema = { + "properties": {"Components": {"properties": {"Flight Controller": {"description": "FC"}}}}, + "definitions": {"product": {"properties": {"Manufacturer": {"type": "string"}}}}, + } + json_schema = VehicleComponentsJsonSchema(schema) + + result = json_schema.get_component_property_description(("Flight Controller", "Product", "NonExistentField")) + + assert result == ("", False) + + def test_get_section_field_description_returns_direct_property_description(self) -> None: + """ + _get_section_field_description returns description when section found in direct properties. + + GIVEN: A schema where 'Flight Controller' has 'Specifications' in its direct properties + WHEN: get_component_property_description is called with path ('Flight Controller', 'Specifications') + THEN: The description from the direct property is returned (line 240) + """ + fc_body = {"description": "FC", "properties": {"Specifications": {"description": "Hardware specifications"}}} + schema = make_fc_schema(fc_body) + json_schema = VehicleComponentsJsonSchema(schema) + + result = json_schema.get_component_property_description(("Flight Controller", "Specifications")) + + assert result == ("Hardware specifications", False) + + def test_get_section_field_description_finds_section_via_ref_in_allof(self) -> None: + """ + _get_section_field_description returns description when section found via $ref inside allOf. + + GIVEN: A schema where 'Flight Controller' has an allOf with a $ref, and the referenced + definition contains the target section in its properties + WHEN: get_component_property_description is called with path ('Flight Controller', 'Firmware') + THEN: The description from the resolved reference is returned (line 250) + """ + schema = { + "properties": {"Components": {"properties": {"Flight Controller": {"allOf": [{"$ref": "#/definitions/fcBase"}]}}}}, + "definitions": { + "fcBase": {"properties": {"Firmware": {"description": "Firmware section", "x-is-optional": True}}} + }, + } + json_schema = VehicleComponentsJsonSchema(schema) + + result = json_schema.get_component_property_description(("Flight Controller", "Firmware")) + + assert result == ("Firmware section", True) + + def test_modify_schema_logs_error_when_allof_has_no_specifications(self) -> None: + """ + modify_schema_for_mcu_series logs an error when allOf items lack Specifications. + + GIVEN: A schema where flightController.allOf exists but no item contains 'Specifications' + WHEN: modify_schema_for_mcu_series is called + THEN: An error should be logged ('Could not find Specifications ...') + and the function should return early without modifying anything + """ + schema_no_specs = { + "definitions": {"flightController": {"allOf": [{"properties": {"NotSpecifications": {"type": "string"}}}]}} + } + json_schema = VehicleComponentsJsonSchema(schema_no_specs) + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_json_schema.logging_error") as mock_err: + json_schema.modify_schema_for_mcu_series(is_optional=True) + + mock_err.assert_called() + + def test_modify_schema_does_nothing_when_specifications_has_no_mcu_series_property(self) -> None: + """ + modify_schema_for_mcu_series exits silently when MCU Series is absent from Specifications. + + GIVEN: A schema where flightController references Specifications, but Specifications + has no 'properties' key (or no 'MCU Series' entry) + WHEN: modify_schema_for_mcu_series is called + THEN: No error should be logged and the function should complete without modification + """ + schema_no_mcu = { + "definitions": { + "flightController": { + "allOf": [ + { + "properties": { + "Specifications": {"type": "object"} # no 'properties' sub-key + } + } + ] + } + } + } + json_schema = VehicleComponentsJsonSchema(schema_no_mcu) + + with patch("ardupilot_methodic_configurator.data_model_vehicle_components_json_schema.logging_error") as mock_err: + json_schema.modify_schema_for_mcu_series(is_optional=True) + + mock_err.assert_not_called() + + def test_get_component_property_description_returns_empty_for_unknown_top_level_component(self) -> None: + """ + get_component_property_description returns empty tuple for an unknown top-level component. + + GIVEN: A schema without 'NonExistentComponent' in its Components properties + WHEN: get_component_property_description is called with a single-element path + THEN: ('', False) should be returned + """ + schema = {"properties": {"Components": {"properties": {"Flight Controller": {"description": "FC description"}}}}} + json_schema = VehicleComponentsJsonSchema(schema) + + result = json_schema.get_component_property_description(("NonExistentComponent",)) + + assert result == ("", False) + + def test_get_component_property_description_returns_empty_for_unknown_component_in_nested_path(self) -> None: + """ + get_component_property_description returns empty tuple for unknown component in nested path. + + The top-level component is not found while resolving a multi-element path. + + GIVEN: A schema without 'NonExistent' as a component type + WHEN: get_component_property_description is called with a path starting with 'NonExistent' + THEN: ('', False) should be returned + """ + schema = {"properties": {"Components": {"properties": {"Flight Controller": {"description": "FC description"}}}}} + json_schema = VehicleComponentsJsonSchema(schema) + + result = json_schema.get_component_property_description(("NonExistent", "Specifications", "MCU Series")) + + assert result == ("", False) + + def test_get_component_property_description_returns_empty_for_missing_section(self) -> None: + """ + get_component_property_description returns empty tuple for a missing section path. + + When len(path)==2 and the section is not in the component schema, ('', False) is returned. + + GIVEN: A schema with 'Flight Controller' in Components.properties + but no 'NonExistentSection' anywhere + WHEN: get_component_property_description is called with path of length 2 + THEN: ('', False) should be returned from _get_section_field_description + """ + schema = { + "properties": { + "Components": { + "properties": { + "Flight Controller": { + "description": "FC", + "properties": {"Product": {"description": "Product info"}}, + } + } + } + } + } + json_schema = VehicleComponentsJsonSchema(schema) + + result = json_schema.get_component_property_description(("Flight Controller", "NonExistentSection")) + + assert result == ("", False) + + def test_traverse_nested_path_returns_empty_when_part_not_found_in_any_strategy(self) -> None: + """ + _traverse_nested_path returns ('', False) when a path part is not found. + + No lookup strategy (direct, allOf, or ref) locates the requested property. + Also exercises: 243->257 (len(path)!=2 branch) and 289->302 (no allOf in schema_obj). + + GIVEN: A schema where 'Flight Controller' has only a 'Product' property + WHEN: get_component_property_description is called with a length-3 path whose + second element is 'Specifications' (which does not exist) + THEN: _traverse_nested_path should exhaust all strategies and return ('', False) + """ + schema = { + "properties": { + "Components": { + "properties": { + "Flight Controller": { + "description": "FC", + "properties": {"Product": {"description": "Product section"}}, + } + } + } + } + } + json_schema = VehicleComponentsJsonSchema(schema) + + # path length 3 with path[1] != 'Product' -> goes to _traverse_nested_path + result = json_schema.get_component_property_description(("Flight Controller", "Specifications", "Processor")) + + assert result == ("", False) + + def test_check_allof_constructs_finds_property_via_ref_inside_allof_item(self) -> None: + """ + _check_allof_constructs returns True when the target property is found via $ref in allOf. + + The first allOf item lacks the property but the second resolves via $ref to a definition + that contains it. + + GIVEN: An allOf schema where the first item doesn't have the property + but the second item is a $ref that resolves to a definition containing it + WHEN: _check_allof_constructs is called for the target property + THEN: Should return True and the property schema + """ + schema = { + "definitions": {"myDef": {"properties": {"TargetProp": {"type": "string", "description": "Found via $ref"}}}} + } + json_schema = VehicleComponentsJsonSchema(schema) + + test_obj = { + "allOf": [ + {"properties": {"OtherProp": {"type": "integer"}}}, # doesn't have TargetProp + {"$ref": "#/definitions/myDef"}, # resolves to definition with TargetProp + ] + } + + found, result = json_schema._check_allof_constructs(test_obj, "TargetProp") + + assert found is True + assert result["type"] == "string" + + def test_check_references_finds_property_in_direct_properties_of_referenced_definition(self) -> None: + """ + _check_references returns True for a target property in referenced definition's properties. + + The $ref resolves to a definition with direct properties (not via allOf). + + GIVEN: A schema where the $ref resolves to a definition with direct properties + containing the target property + WHEN: _check_references is called for the target property + THEN: Should return True and the property schema + """ + schema = { + "definitions": { + "myDirectDef": {"properties": {"DirectProp": {"type": "boolean", "description": "Direct property"}}} + } + } + json_schema = VehicleComponentsJsonSchema(schema) + + test_obj = {"$ref": "#/definitions/myDirectDef"} + + found, result = json_schema._check_references(test_obj, "DirectProp") + + assert found is True + assert result["type"] == "boolean"