feat(vehicle-components): add BATT_ARM_VOLT, MOT_BAT_VOLT_MIN per-cell support

add battery voltage path Volt per cell arm/min in validation constants and schema path coverage
add regression tests for arm/min voltage conversions

Author: amilcarlucas

ardupilot_methodic_configurator/battery_cell_voltages.py

@@ -15,57 +15,73 @@
         "absolute_max": 4.2,
         "absolute_min": 2.5,
         "recommended_max": 4.1,
+        "recommended_arm": 3.6,
         "recommended_low": 3.1,
         "recommended_crit": 2.8,
+        "recommended_min": 2.7,
     },
     "LiIonSS": {
         "absolute_max": 4.2,
         "absolute_min": 2.4,
         "recommended_max": 4.2,
+        "recommended_arm": 3.6,
         "recommended_low": 3.0,
         "recommended_crit": 2.7,
+        "recommended_min": 2.6,
     },
     "LiIonSSHV": {
         "absolute_max": 4.45,
         "absolute_min": 2.4,
         "recommended_max": 4.45,
+        "recommended_arm": 3.8,
         "recommended_low": 3.0,
         "recommended_crit": 2.7,
+        "recommended_min": 2.6,
     },
     "Lipo": {
         "absolute_max": 4.2,
         "absolute_min": 3.0,
         "recommended_max": 4.2,
+        "recommended_arm": 3.8,
         "recommended_low": 3.6,
         "recommended_crit": 3.3,
+        "recommended_min": 3.2,
     },
     "LipoHV": {
         "absolute_max": 4.35,
         "absolute_min": 3.0,
         "recommended_max": 4.35,
+        "recommended_arm": 3.9,
         "recommended_low": 3.6,
         "recommended_crit": 3.3,
+        "recommended_min": 3.2,
     },
     "LipoHVSS": {
         "absolute_max": 4.2,
         "absolute_min": 2.9,
         "recommended_max": 4.2,
+        "recommended_arm": 3.8,
         "recommended_low": 3.5,
         "recommended_crit": 3.2,
+        "recommended_min": 3.1,
     },
     "NiCd": {
         "absolute_max": 1.45,
         "absolute_min": 1.0,
         "recommended_max": 1.4,
+        "recommended_arm": 1.28,
         "recommended_low": 1.2,
         "recommended_crit": 1.1,
+        "recommended_min": 1.05,
     },
     "NiMH": {
         "absolute_max": 1.45,
         "absolute_min": 1.0,
         "recommended_max": 1.4,
+        "recommended_arm": 1.28,
         "recommended_low": 1.2,
         "recommended_crit": 1.1,
+        "recommended_min": 1.05,
     },
     # Add more chemistries as needed
 }
@@ -101,16 +117,28 @@ def limit_min_voltage(chemistry: str) -> float:
     def recommended_max_voltage(chemistry: str) -> float:
         if chemistry not in battery_cell_voltages:
             return nan
-        return battery_cell_voltages[chemistry].get("recommended_max", 4.2)
+        return battery_cell_voltages[chemistry].get("recommended_max", nan)
+
+    @staticmethod
+    def recommended_arm_voltage(chemistry: str) -> float:
+        if chemistry not in battery_cell_voltages:
+            return nan
+        return battery_cell_voltages[chemistry].get("recommended_arm", nan)
 
     @staticmethod
     def recommended_low_voltage(chemistry: str) -> float:
         if chemistry not in battery_cell_voltages:
             return nan
-        return battery_cell_voltages[chemistry].get("recommended_low", 3.6)
+        return battery_cell_voltages[chemistry].get("recommended_low", nan)
 
     @staticmethod
     def recommended_crit_voltage(chemistry: str) -> float:
         if chemistry not in battery_cell_voltages:
             return nan
-        return battery_cell_voltages[chemistry].get("recommended_crit", 3.3)
+        return battery_cell_voltages[chemistry].get("recommended_crit", nan)
+
+    @staticmethod
+    def recommended_min_voltage(chemistry: str) -> float:
+        if chemistry not in battery_cell_voltages:
+            return nan
+        return battery_cell_voltages[chemistry].get("recommended_min", nan)

ardupilot_methodic_configurator/configuration_steps_ArduCopter.json

@@ -129,14 +129,14 @@
                 "BATT_FS_LOW_ACT": { "New Value": 2, "Change Reason": "Return and land at home or rally point" }
             },
             "derived_parameters": {
-                "BATT_ARM_VOLT": { "New Value": "(vehicle_components['Battery']['Specifications']['Number of cells']-1)*0.1+(vehicle_components['Battery']['Specifications']['Volt per cell crit']+0.3)*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "Do not allow arming below this voltage" },
+                "BATT_ARM_VOLT": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell arm']+0.0)*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "Only arm above this voltage, to avoid taking off with insufficient battery capacity" },
                 "BATT_CAPACITY": { "New Value": "(vehicle_components['Battery']['Specifications']['Capacity mAh'])", "Change Reason": "Total battery capacity specified in the component editor" },
                 "BATT_CRT_VOLT": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell crit'])*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "(Critical voltage + 0.0) x no. of cells" },
                 "BATT_LOW_VOLT": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell low'])*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "(Low voltage + 0.0) x no. of cells" },
                 "BATT_MONITOR": { "New Value": "vehicle_components['Battery Monitor']['FC Connection']['Protocol']", "Change Reason": "Selected in component editor window" },
                 "BATT_I2C_BUS": { "New Value": "1 if vehicle_components['Battery Monitor']['FC Connection']['Type'] == 'I2C2' else 2 if vehicle_components['Battery Monitor']['FC Connection']['Type'] == 'I2C3' else 3 if vehicle_components['Battery Monitor']['FC Connection']['Type'] == 'I2C4' else 0", "Change Reason": "Selected in component editor window" },
-                "MOT_BAT_VOLT_MAX": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell max']+0.0)*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "(Max voltage + 0.0) x no. of cells" },
-                "MOT_BAT_VOLT_MIN": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell crit']+0.0)*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "(Critical voltage + 0.0) x no. of cells" }
+                "MOT_BAT_VOLT_MAX": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell max']+0.0)*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "Scale the PIDs up when battery voltage is below this threshold" },
+                "MOT_BAT_VOLT_MIN": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell min']+0.0)*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "Scale the PIDs up when battery voltage is above this threshold" }
             },
             "rename_connection": "vehicle_components['Battery Monitor']['FC Connection']['Type']",
             "old_filenames": [],

ardupilot_methodic_configurator/data_model_vehicle_components_import.py

@@ -9,10 +9,12 @@
 """
 
 import contextlib
+from dataclasses import dataclass
 
 # from logging import debug as logging_debug
 from logging import error as logging_error
 from logging import warning as logging_warning
+from math import isnan, nan
 from typing import Any, Optional
 
 from ardupilot_methodic_configurator import _
@@ -55,6 +57,17 @@ def is_single_bit_set(value: int) -> bool:
     return value > 0 and value & (value - 1) == 0
 
 
+@dataclass
+class BatteryVoltageSpecs:
+    """Data class to hold battery voltage specifications for import processing."""
+
+    estimated_cell_count: int
+    limit_min: float
+    limit_max: float
+    detected_chemistry: str
+    fc_parameters: dict[str, float]
+
+
 class ComponentDataModelImport(ComponentDataModelBase):
     """
     A class to handle component data import from FC parameters separate from UI logic.
@@ -371,6 +384,30 @@ def _set_battery_type_from_fc_parameters(self, fc_parameters: dict[str, float])
         else:
             logging_warning(_("BATT_MONITOR parameter not found in fc_parameters"))
 
+        # Detect battery chemistry from voltage parameters before estimating cell count
+        # Only update if current chemistry is clearly wrong or not set
+        current_chemistry = str(self.get_component_value(("Battery", "Specifications", "Chemistry")))
+        detected_chemistry = self._detect_battery_chemistry_from_voltages(fc_parameters, current_chemistry)
+        if detected_chemistry and detected_chemistry != current_chemistry:
+            self.set_component_value(("Battery", "Specifications", "Chemistry"), detected_chemistry)
+
+        # if chemistry detection fails, use current chemistry if valid, otherwise default to "Lipo"
+        final_chemistry = detected_chemistry or current_chemistry or "Lipo"
+        if final_chemistry not in BatteryCell.chemistries():
+            final_chemistry = "Lipo"
+
+        specs = BatteryVoltageSpecs(
+            estimated_cell_count=self._estimate_battery_cell_count(fc_parameters),
+            limit_min=BatteryCell.limit_min_voltage(final_chemistry),
+            limit_max=BatteryCell.limit_max_voltage(final_chemistry),
+            detected_chemistry=final_chemistry,
+            fc_parameters=fc_parameters,
+        )
+        self._import_bat_values_from_fc(specs)
+
+    def _import_bat_values_from_fc(self, specs: BatteryVoltageSpecs) -> None:
+        fc_parameters = specs.fc_parameters
+
         if "BATT_CAPACITY" in fc_parameters:
             try:
                 batt_capacity = int(fc_parameters["BATT_CAPACITY"])
@@ -383,34 +420,54 @@ def _set_battery_type_from_fc_parameters(self, fc_parameters: dict[str, float])
         else:
             logging_warning(_("BATT_CAPACITY parameter not found in fc_parameters"))
 
-        # Detect battery chemistry from voltage parameters before estimating cell count
-        # Only update if current chemistry is clearly wrong or not set
-        current_chemistry = str(self.get_component_value(("Battery", "Specifications", "Chemistry")))
-        detected_chemistry = self._detect_battery_chemistry_from_voltages(fc_parameters, current_chemistry)
-        if detected_chemistry and detected_chemistry != current_chemistry:
-            self.set_component_value(("Battery", "Specifications", "Chemistry"), detected_chemistry)
+        if specs.estimated_cell_count > 0:
+            self.import_bat_voltage(specs, "MOT_BAT_VOLT_MAX", "max")
+            self.import_bat_voltage(specs, "BATT_ARM_VOLT", "arm")
+            self.import_bat_voltage(specs, "BATT_LOW_VOLT", "low")
+            self.import_bat_voltage(specs, "BATT_CRT_VOLT", "crit")
+            self.import_bat_voltage(specs, "MOT_BAT_VOLT_MIN", "min")
 
-        # Estimate number of cells from voltage parameters
-        self._estimate_battery_cell_count(fc_parameters)
+    def import_bat_voltage(self, specs: BatteryVoltageSpecs, param_name: str, voltage_type: str) -> None:
+        if param_name in specs.fc_parameters:
+            try:
+                cell = specs.fc_parameters[param_name] / specs.estimated_cell_count
+                if specs.limit_min <= cell <= specs.limit_max:
+                    self.set_component_value(("Battery", "Specifications", f"Volt per cell {voltage_type}"), cell)
+                else:
+                    logging_warning(
+                        _("Calculated cell %s voltage %.2f V is out of expected range for %s chemistry (%.2f - %.2f V)"),
+                        voltage_type,
+                        cell,
+                        specs.detected_chemistry,
+                        specs.limit_min,
+                        specs.limit_max,
+                    )
+            except (ValueError, TypeError) as e:
+                logging_error(_("Error processing %s parameter: %s"), param_name, str(e))
 
     def _get_volt_per_cell_for_type(self, chemistry: str, voltage_type: str) -> float:
         """
         Get voltage per cell for a given chemistry and voltage type.
 
         Args:
             chemistry: Battery chemistry type
-            voltage_type: One of "recommended_max", "recommended_low", "recommended_crit"
+            voltage_type: One of "recommended_max", "recommended_arm", "recommended_low", "recommended_crit", "recommended_min"
 
         Returns:
             Voltage per cell value
 
         """
         if voltage_type == "recommended_max":
             return BatteryCell.recommended_max_voltage(chemistry)
+        if voltage_type == "recommended_arm":
+            return BatteryCell.recommended_arm_voltage(chemistry)
         if voltage_type == "recommended_low":
             return BatteryCell.recommended_low_voltage(chemistry)
-        # voltage_type == "recommended_crit"
-        return BatteryCell.recommended_crit_voltage(chemistry)
+        if voltage_type == "recommended_crit":
+            return BatteryCell.recommended_crit_voltage(chemistry)
+        if voltage_type == "recommended_min":
+            return BatteryCell.recommended_min_voltage(chemistry)
+        return nan
 
     def _detect_battery_chemistry_from_voltages(
         self, fc_parameters: dict[str, float], current_chemistry: Optional[str] = None
@@ -431,11 +488,16 @@ def _detect_battery_chemistry_from_voltages(
             or current chemistry is acceptable
 
         """
-        # Priority: MOT_BAT_VOLT_MAX > BATT_LOW_VOLT > BATT_CRT_VOLT
+        # Priority: MOT_BAT_VOLT_MAX > BATT_LOW_VOLT > BATT_CRT_VOLT > BATT_ARM_VOLT > MOT_BAT_VOLT_MIN
         voltage_params = [
             ("MOT_BAT_VOLT_MAX", "recommended_max"),
             ("BATT_LOW_VOLT", "recommended_low"),
             ("BATT_CRT_VOLT", "recommended_crit"),
+            (
+                "BATT_ARM_VOLT",
+                "recommended_arm",
+            ),  # lower priority, less commonly set, but can provide additional clues if available
+            ("MOT_BAT_VOLT_MIN", "recommended_min"),  # lowest priority, but can be a last resort for detection
         ]
 
         # Try each voltage parameter in priority order
@@ -451,22 +513,28 @@ def _detect_battery_chemistry_from_voltages(
                 # Check if current chemistry is consistent with this parameter
                 if current_chemistry and current_chemistry in BatteryCell.chemistries():
                     volt_per_cell = self._get_volt_per_cell_for_type(current_chemistry, voltage_type)
-                    if volt_per_cell > 0:
-                        estimated_cells = total_voltage / volt_per_cell
-                        current_chemistry_score = abs(estimated_cells - round(estimated_cells))
+                    if isnan(volt_per_cell) or volt_per_cell <= 0:
+                        logging_warning(
+                            _("Current chemistry %s does not have valid volt per cell for %s, ignoring current chemistry"),
+                            current_chemistry,
+                            voltage_type,
+                        )
+                        continue
+                    estimated_cells = total_voltage / volt_per_cell
+                    current_chemistry_score = abs(estimated_cells - round(estimated_cells))
 
-                        # If current chemistry gives acceptable results (<40% error), keep it
-                        # This very conservative threshold prevents changing chemistry unless clearly wrong
-                        if current_chemistry_score < 0.40:
-                            return None  # Keep current chemistry
+                    # If current chemistry gives acceptable results (<40% error), keep it
+                    # This very conservative threshold prevents changing chemistry unless clearly wrong
+                    if current_chemistry_score < 0.40:
+                        return None  # Keep current chemistry
 
                 # Find best matching chemistry
                 best_chemistry = None
                 best_score = float("inf")
 
                 for chemistry in BatteryCell.chemistries():
                     volt_per_cell = self._get_volt_per_cell_for_type(chemistry, voltage_type)
-                    if volt_per_cell <= 0:
+                    if isnan(volt_per_cell) or volt_per_cell <= 0:
                         continue
 
                     estimated_cells = total_voltage / volt_per_cell
@@ -520,19 +588,19 @@ def _estimate_cells_from_voltage_param(
 
         return None
 
-    def _estimate_battery_cell_count(self, fc_parameters: dict[str, float]) -> None:
+    def _estimate_battery_cell_count(self, fc_parameters: dict[str, float]) -> int:
         """
         Estimate battery cell count from voltage parameters.
 
-        Uses MOT_BAT_VOLT_MAX, BATT_LOW_VOLT, or BATT_CRT_VOLT along with
-        current volt-per-cell values to estimate the number of cells.
+        Uses MOT_BAT_VOLT_MAX, BATT_LOW_VOLT, BATT_CRT_VOLT, BATT_ARM_VOLT, or MOT_BAT_VOLT_MIN
+        along with current volt-per-cell values to estimate the number of cells.
 
         Args:
             fc_parameters: Dictionary of flight controller parameters
 
         """
         # Try to estimate cell count from available voltage parameters
-        # Priority: MOT_BAT_VOLT_MAX > BATT_LOW_VOLT > BATT_CRT_VOLT
+        # Priority: MOT_BAT_VOLT_MAX > BATT_LOW_VOLT > BATT_CRT_VOLT > BATT_ARM_VOLT > MOT_BAT_VOLT_MIN
         estimated_cells = None
 
         if "MOT_BAT_VOLT_MAX" in fc_parameters:
@@ -550,24 +618,36 @@ def _estimate_battery_cell_count(self, fc_parameters: dict[str, float]) -> None:
                 "BATT_CRT_VOLT", fc_parameters["BATT_CRT_VOLT"], "Volt per cell crit"
             )
 
+        # lower prio, because less often used
+        if estimated_cells is None and "BATT_ARM_VOLT" in fc_parameters:
+            estimated_cells = self._estimate_cells_from_voltage_param(
+                "BATT_ARM_VOLT", fc_parameters["BATT_ARM_VOLT"], "Volt per cell arm"
+            )
+
+        if estimated_cells is None and "MOT_BAT_VOLT_MIN" in fc_parameters:
+            estimated_cells = self._estimate_cells_from_voltage_param(
+                "MOT_BAT_VOLT_MIN", fc_parameters["MOT_BAT_VOLT_MIN"], "Volt per cell min"
+            )
+
         # If no estimation succeeded, all volt per cell values must be invalid
         if estimated_cells is None:
             logging_error(_("All volt per cell values are zero or invalid; cannot estimate battery cell count"))
-            return
+            return 0
 
         # Validate and set the estimated cell count
         cell_path = ("Battery", "Specifications", "Number of cells")
         if cell_path in ComponentDataModelValidation.VALIDATION_RULES:
             _type, (min_cells, max_cells), _doc = ComponentDataModelValidation.VALIDATION_RULES[cell_path]
             if min_cells <= estimated_cells <= max_cells:
                 self.set_component_value(cell_path, estimated_cells)
-            else:
-                logging_error(
-                    _("Estimated battery cell count %s is out of valid range (%d to %d)"),
-                    estimated_cells,
-                    min_cells,
-                    max_cells,
-                )
+                return estimated_cells
+            logging_error(
+                _("Estimated battery cell count %s is out of valid range (%d to %d)"),
+                estimated_cells,
+                min_cells,
+                max_cells,
+            )
+        return 0
 
     def _set_motor_poles_from_fc_parameters(self, fc_parameters: dict[str, float]) -> None:
         """Process motor parameters and update the data model."""