feat(rover support): Improve support for Rover ESCs

Improve Heli support as well

Author: amilcarlucas

ardupilot_methodic_configurator/annotate_params.py

@@ -580,6 +580,7 @@ def get_fallback_xml_url(vehicle_type: str, firmware_version: str) -> str:
         "ArduPlane": "Plane-",
         "Rover": "Rover-",
         "ArduSub": "Sub-",
+        "Heli": "Copter-",
     }
     try:
         vehicle_subdir = vehicle_parm_subdir[vehicle_type] + firmware_version[0:3]

ardupilot_methodic_configurator/configuration_steps_Rover.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_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_ARM_VOLT": { "New Value": "vehicle_components['Battery']['Specifications']['Volt per cell arm']*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 failsafe voltage x nr. 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 failsafe voltage x nr. 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']*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']*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": [],
@@ -170,7 +170,8 @@
             "mandatory_text": "100% mandatory (0% optional)",
             "auto_changed_by": "",
             "derived_parameters": {
-                "GPS_TYPE": { "New Value": "vehicle_components['GNSS Receiver']['FC Connection']['Protocol']", "Change Reason": "Defined in component editor" }
+                "GPS_TYPE": { "New Value": "vehicle_components['GNSS Receiver']['FC Connection']['Protocol']", "Change Reason": "Defined in component editor" },
+                "GPS1_TYPE": { "New Value": "vehicle_components['GNSS Receiver']['FC Connection']['Protocol']", "Change Reason": "Defined in component editor" }
             },
             "rename_connection": "vehicle_components['GNSS Receiver']['FC Connection']['Type']",
             "old_filenames": []
@@ -269,7 +270,11 @@
             "external_tool_url": "",
             "mandatory_text": "40% mandatory (60% optional)",
             "auto_changed_by": "",
-            "old_filenames": ["14_motor.param"]
+            "old_filenames": ["14_motor.param"],
+            "plugin": {
+                "name": "motor_test",
+                "placement": "left"
+            }
         },
         "16_pid_adjustment.param": {
             "why": "With very large or very small vehicles the default PID values are not suitable for the first flight",

ardupilot_methodic_configurator/data_model_vehicle_components_import.py

@@ -30,11 +30,11 @@
     CAN_PORTS,
     GNSS_RECEIVER_CONNECTION,
     I2C_PORTS,
-    MOT_PWM_TYPE_DICT,
     RC_PROTOCOLS_DICT,
     SERIAL_PORTS,
     SERIAL_PROTOCOLS_DICT,
     ComponentDataModelValidation,
+    get_mot_pwm_type_sub_dict,
 )
 
 
@@ -139,6 +139,21 @@ def _verify_dict_is_uptodate(
             if check_key in dict_to_check:
                 code_protocol = dict_to_check[check_key].get("protocol", None)
                 if code_protocol != doc_protocol:
+                    if code_protocol == "Septentrio(SBF)" and doc_protocol == "SBF":
+                        # ArduPilot 4.6 renamed "SBF" to "Septentrio(SBF)"
+                        continue
+                    if code_protocol == "Trimble(GSOF)" and doc_protocol == "GSOF":
+                        # ArduPilot 4.6 renamed "GSOF" to "Trimble(GSOF)"
+                        continue
+                    if code_protocol == "MAVLink" and doc_protocol == "MAV":
+                        # ArduPilot 4.6 renamed "MAV" to "MAVLink"
+                        continue
+                    if code_protocol == "Septentrio-DualAntenna(SBF)" and doc_protocol == "SBF-DualAntenna":
+                        # ArduPilot 4.6 renamed "SBF-DualAntenna" to "Septentrio-DualAntenna(SBF)"
+                        continue
+                    if code_protocol == "Gimbal" and doc_protocol == "SToRM32 Gimbal Serial":
+                        # ArduPilot 4.6 renamed "SToRM32 Gimbal Serial" to "Gimbal"
+                        continue
                     logging_warning(_("Protocol %s does not match %s in %s metadata"), code_protocol, doc_protocol, doc_key)
                     is_valid = False
             else:
@@ -166,7 +181,8 @@ def process_fc_parameters(
             self._verify_dict_is_uptodate(doc, GNSS_RECEIVER_CONNECTION, "GPS1_TYPE", "values")
         elif "GPS_TYPE" in doc:
             self._verify_dict_is_uptodate(doc, GNSS_RECEIVER_CONNECTION, "GPS_TYPE", "values")
-        self._verify_dict_is_uptodate(doc, MOT_PWM_TYPE_DICT, "MOT_PWM_TYPE", "values")
+        fw_type = str(self.get_component_value(("Flight Controller", "Firmware", "Type")) or "")
+        self._verify_dict_is_uptodate(doc, get_mot_pwm_type_sub_dict(fw_type), "MOT_PWM_TYPE", "values")
         self._verify_dict_is_uptodate(doc, RC_PROTOCOLS_DICT, "RC_PROTOCOLS", "Bitmask")
 
         # Process frame information first (if available)
@@ -264,7 +280,11 @@ def _set_serial_type_from_fc_parameters(  # pylint: disable=too-many-branches,to
         rc = 1
         telem = 1
         gnss = 1
+        # esc counts FC->ESC *control* serial protocols only (FETtecOneWire, Torqeedo, CoDevESC).
+        # Telemetry-only protocols (ESC Telemetry=16, Scripting=28) do NOT count here because
+        # FC->ESC is still PWM/DShot/BDShot in those cases and _set_esc_type_from_fc_parameters must run.
         esc = 1
+        esc_telemetry_set = False  # True once ESC->FC Telemetry is populated from a serial port
         for serial in SERIAL_PORTS:
             if serial + "_PROTOCOL" not in fc_parameters:
                 continue
@@ -306,14 +326,23 @@ def _set_serial_type_from_fc_parameters(  # pylint: disable=too-many-branches,to
                     self.set_component_value(("GNSS Receiver", "FC Connection", "Type"), serial)
                 gnss += 1
             elif component == "ESC":
-                if esc == 1:
-                    # Only set component values for the first ESC
-                    self.set_component_value(("ESC", "FC->ESC Connection", "Type"), serial)
-                    self.set_component_value(("ESC", "FC->ESC Connection", "Protocol"), protocol)
-                    self.set_component_value(("ESC", "ESC->FC Telemetry", "Type"), serial)
-                    self.set_component_value(("ESC", "ESC->FC Telemetry", "Protocol"), protocol)
-                # Count all ESC components
-                esc += 1
+                if protocol in {"ESC Telemetry", "Scripting"}:
+                    # Serial ESC->FC telemetry only (DShot with UART feedback, or Hobbywing Datalink v2).
+                    # FC->ESC connection is still PWM/DShot; _set_esc_type_from_fc_parameters handles it.
+                    # Do NOT increment esc so that function is still called.
+                    if not esc_telemetry_set:
+                        self.set_component_value(("ESC", "ESC->FC Telemetry", "Type"), serial)
+                        self.set_component_value(("ESC", "ESC->FC Telemetry", "Protocol"), protocol)
+                        esc_telemetry_set = True
+                else:
+                    # FC->ESC serial control protocol (FETtecOneWire, Torqeedo, CoDevESC):
+                    # the same serial port carries both directions.
+                    if esc == 1:
+                        self.set_component_value(("ESC", "FC->ESC Connection", "Type"), serial)
+                        self.set_component_value(("ESC", "FC->ESC Connection", "Protocol"), protocol)
+                        self.set_component_value(("ESC", "ESC->FC Telemetry", "Type"), serial)
+                        self.set_component_value(("ESC", "ESC->FC Telemetry", "Protocol"), protocol)
+                    esc += 1
 
         return esc >= 2
 
@@ -326,29 +355,38 @@ def _set_esc_type_from_fc_parameters(self, fc_parameters: dict[str, float], doc:
             logging_error(_("Invalid non-integer value for MOT_PWM_TYPE %s"), mot_pwm_type)
             mot_pwm_type = 0
 
-        main_out_functions = [fc_parameters.get("SERVO" + str(i) + "_FUNCTION", 0) for i in range(1, 9)]
+        aio_functions = [fc_parameters.get("SERVO" + str(i) + "_FUNCTION", 0) for i in range(9, 15)]
 
-        # if any element of main_out_functions is in [33, 34, 35, 36] then ESC is connected to main_out
-        if any(servo_function in {33, 34, 35, 36} for servo_function in main_out_functions):
-            self.set_component_value(("ESC", "FC->ESC Connection", "Type"), "Main Out")
-        else:
+        # if any element of aio_functions is in [33, 34, 35, 36, 73, 74] then ESC is connected to AIO
+        if any(servo_function in {33, 34, 35, 36, 73, 74} for servo_function in aio_functions):
             self.set_component_value(("ESC", "FC->ESC Connection", "Type"), "AIO")
+        else:
+            self.set_component_value(("ESC", "FC->ESC Connection", "Type"), "Main Out")
 
         protocol = ""
         if "MOT_PWM_TYPE" in doc and "values" in doc["MOT_PWM_TYPE"]:
             protocol = doc["MOT_PWM_TYPE"]["values"].get(str(mot_pwm_type), "")
             if protocol:
                 self.set_component_value(("ESC", "FC->ESC Connection", "Protocol"), protocol)
         # Fallback to MOT_PWM_TYPE_DICT if doc is not available
-        elif str(mot_pwm_type) in MOT_PWM_TYPE_DICT:
-            protocol = str(MOT_PWM_TYPE_DICT[str(mot_pwm_type)]["protocol"])
-            self.set_component_value(("ESC", "FC->ESC Connection", "Protocol"), protocol)
+        else:
+            mot_pwm_sub = get_mot_pwm_type_sub_dict(
+                str(self.get_component_value(("Flight Controller", "Firmware", "Type")) or "")
+            )
+            if str(mot_pwm_type) in mot_pwm_sub:
+                protocol = str(mot_pwm_sub[str(mot_pwm_type)]["protocol"])
+                self.set_component_value(("ESC", "FC->ESC Connection", "Protocol"), protocol)
 
-        # Set ESC->FC Telemetry: DShot protocols support BDShot telemetry on the same PWM wire
+        # Set ESC->FC Telemetry: DShot supports BDShot telemetry on the same PWM wire.
+        # However, if _set_serial_type_from_fc_parameters already found a dedicated serial telemetry
+        # port (SERIAL*_PROTOCOL=16 or 28), that takes priority and must not be overwritten.
         esc_conn_type = self.get_component_value(("ESC", "FC->ESC Connection", "Type"))
         if protocol and protocol.startswith("DShot"):
-            self.set_component_value(("ESC", "ESC->FC Telemetry", "Type"), esc_conn_type)
-            self.set_component_value(("ESC", "ESC->FC Telemetry", "Protocol"), "BDShot")
+            current_telemetry_type = self.get_component_value(("ESC", "ESC->FC Telemetry", "Type"))
+            if current_telemetry_type not in SERIAL_PORTS:
+                # No dedicated serial telemetry port detected; fall back to BDShot on the PWM wire
+                self.set_component_value(("ESC", "ESC->FC Telemetry", "Type"), esc_conn_type)
+                self.set_component_value(("ESC", "ESC->FC Telemetry", "Protocol"), "BDShot")
         else:
             self.set_component_value(("ESC", "ESC->FC Telemetry", "Type"), "None")
             self.set_component_value(("ESC", "ESC->FC Telemetry", "Protocol"), "None")
@@ -447,6 +485,12 @@ def _import_bat_values_from_fc(self, specs: BatteryVoltageSpecs) -> None:
             self.import_bat_voltage(specs, "BATT_LOW_VOLT", "Volt per cell low")
             self.import_bat_voltage(specs, "BATT_CRT_VOLT", "Volt per cell crit")
             self.import_bat_voltage(specs, "MOT_BAT_VOLT_MIN", "Volt per cell min")
+        else:
+            self.set_component_value(("Battery", "Specifications", "Volt per cell max"), "0")
+            self.set_component_value(("Battery", "Specifications", "Volt per cell arm"), "0")
+            self.set_component_value(("Battery", "Specifications", "Volt per cell low"), "0")
+            self.set_component_value(("Battery", "Specifications", "Volt per cell crit"), "0")
+            self.set_component_value(("Battery", "Specifications", "Volt per cell min"), "0")
 
     def import_bat_voltage(self, specs: BatteryVoltageSpecs, param_name: str, voltage_type: str) -> None:
         if param_name in specs.fc_parameters:
@@ -636,7 +680,10 @@ def _set_motor_poles_from_fc_parameters(self, fc_parameters: dict[str, float]) -
         poles = 0.0
         if "MOT_PWM_TYPE" in fc_parameters:
             mot_pwm_type_str = str(int(fc_parameters["MOT_PWM_TYPE"]))
-            if mot_pwm_type_str in MOT_PWM_TYPE_DICT and MOT_PWM_TYPE_DICT[mot_pwm_type_str].get("is_dshot", False):
+            mot_pwm_sub = get_mot_pwm_type_sub_dict(
+                str(self.get_component_value(("Flight Controller", "Firmware", "Type")) or "")
+            )
+            if mot_pwm_type_str in mot_pwm_sub and mot_pwm_sub[mot_pwm_type_str].get("is_dshot", False):
                 if fc_parameters.get("SERVO_BLH_POLES"):  # DShot ESCs
                     poles = fc_parameters["SERVO_BLH_POLES"]
             elif fc_parameters.get("SERVO_FTW_MASK") and fc_parameters.get("SERVO_FTW_POLES"):