Motor test gui #700

amilcarlucas · 2025-08-15T12:36:28Z

No description provided.

github-actions · 2025-08-15T12:38:07Z

Test Results

0 files - 2 0 suites - 2 0s ⏱️ - 1m 17s
0 tests - 1 565 0 ✅ - 1 564 0 💤 - 1 0 ❌ ±0
0 runs - 3 130 0 ✅ - 3 128 0 💤 - 2 0 ❌ ±0

Results for commit 260b790. ± Comparison against base commit 8177be9.

♻️ This comment has been updated with latest results.

Copilot

Pull Request Overview

This PR introduces a comprehensive motor test GUI for ArduPilot flight controllers, implementing a complete Model-View architecture with robust testing capabilities. The implementation adds motor testing functionality, frame configuration management, battery safety monitoring, and SVG diagram rendering through new GUI components and data models.

Motor test GUI implementation with safety features and real-time feedback
Exception-based error handling refactoring in data model with comprehensive validation
SVG conversion utilities for motor diagrams with multiple backend support

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tests/test_data_model_motor_test.py	Updated tests to use exception-based error handling and properties instead of method calls
scripts/download_motor_diagrams.py	Added SVG to PNG conversion functionality with multiple backend support
pyproject.toml	Added scikit-build and tksvg dependencies for GUI functionality
ardupilot_methodic_configurator/frontend_tkinter_motor_test.py	New comprehensive motor test GUI implementation with safety features
ardupilot_methodic_configurator/data_model_motor_test.py	Major refactoring to exception-based error handling and property-based API
ardupilot_methodic_configurator/backend_flightcontroller.py	Added fetch_param method for parameter verification
ardupilot_methodic_configurator/backend_filesystem_program_settings.py	Restructured motor test settings to nested configuration
ARCHITECTURE_motor_test.md	Updated documentation to reflect implemented features
.vscode/launch.json	Added debug configuration for motor test frontend

Copilot · 2025-08-15T12:38:13Z

scripts/download_motor_diagrams.py

 if __name__ == "__main__":
-    download_motor_diagrams()
+    # download_motor_diagrams()
+    convert_svg_to_png(result_height=320)


The commented out function call suggests this is development/testing code. Consider either removing this comment or making it conditional based on command-line arguments for cleaner production code.

Suggested change

convert_svg_to_png(result_height=320)

parser = argparse.ArgumentParser(

description="Download and/or convert ArduPilot motor diagram SVG files."

)

parser.add_argument(

"--download",

action="store_true",

help="Download all motor diagram SVG files."

)

parser.add_argument(

"--convert",

action="store_true",

help="Convert all SVG files to PNG."

)

parser.add_argument(

"--height",

type=int,

default=320,

help="Resulting PNG height in pixels (default: 320)."

)

args = parser.parse_args()

if args.download:

download_motor_diagrams()

if args.convert:

convert_svg_to_png(result_height=args.height)

if not args.download and not args.convert:

parser.print_help()

Copilot · 2025-08-15T12:38:13Z

ardupilot_methodic_configurator/frontend_tkinter_motor_test.py

+                self._diagrams_path = diagram_path
+
+            if diagram_path and diagram_path.endswith(".svg"):
+                logging_error(_("Found SVG diagram at: %(path)s"), {"path": diagram_path})


Using logging_error for a successful operation is incorrect. This should be logging_info or logging_debug since finding the SVG diagram is expected behavior, not an error condition.

Suggested change

logging_error(_("Found SVG diagram at: %(path)s"), {"path": diagram_path})

logging_info(_("Found SVG diagram at: %(path)s"), {"path": diagram_path})

Copilot · 2025-08-15T12:38:13Z

ardupilot_methodic_configurator/frontend_tkinter_motor_test.py

+                    self.diagram_canvas.create_text(200, 150, text=_("Error loading diagram"), fill="red")
+            else:
+                # Fallback: just show the path
+                logging_error(error_msg)


Consider including the original exception in the logging call for better debugging information. Use logging_error with exception details or logging.exception() to include traceback.

Copilot · 2025-08-15T12:38:14Z

ardupilot_methodic_configurator/data_model_motor_test.py

            ProgramSettings.set_setting("motor_test/duration", duration)
-            return True
+            self._test_duration_s = int(duration)


Converting duration to int loses precision. The duration should remain as a float since it can have decimal values (e.g., 2.5 seconds), but the property getter returns int which is inconsistent.

Suggested change

self._test_duration_s = int(duration)

self._test_duration_s = float(duration)

Copilot · 2025-08-15T12:38:14Z

ardupilot_methodic_configurator/data_model_motor_test.py

+            int: Throttle percentage (1-100)
+
+        """
+        return int(self._test_throttle_pct)


Consider storing throttle_pct as int consistently throughout the class instead of converting on every access. The internal storage as float but return as int creates unnecessary type conversions.

Suggested change

return int(self._test_throttle_pct)

return self._test_throttle_pct

Copilot · 2025-08-15T12:38:15Z

ardupilot_methodic_configurator/backend_flightcontroller.py

+                    return float(msg.param_value)
+            time_sleep(0.01)  # Small sleep to prevent busy waiting
+
+        raise TimeoutError(_("Timeout waiting for parameter %s") % param_name)


The error message should include the timeout duration for better debugging. Consider changing to: 'Timeout waiting for parameter %s after %d seconds' to provide more context.

Suggested change

raise TimeoutError(_("Timeout waiting for parameter %s") % param_name)

raise TimeoutError(_("Timeout waiting for parameter %s after %d seconds") % (param_name, timeout))

Copilot · 2025-08-15T12:38:16Z

tests/test_data_model_motor_test.py

            patch.object(MotorTestDataModel, "_update_frame_configuration", side_effect=Exception("Test exception")),
-            # Act & Assert: Exception should be caught and re-raised
            pytest.raises(Exception, match="Test exception"),


[nitpick] The context manager syntax is split across lines unnecessarily. Consider reformatting for better readability by keeping the opening parenthesis on the same line as the first context manager.

Suggested change

pytest.raises(Exception, match="Test exception"),

with (patch.object(MotorTestDataModel, "_update_frame_configuration", side_effect=Exception("Test exception")),

pytest.raises(Exception, match="Test exception"),

…rror_messages in tuples. This is more pythonic

Required because TKinter can not display .svg files

for more information, see https://pre-commit.ci

Copilot AI review requested due to automatic review settings August 15, 2025 12:36

Copilot AI reviewed Aug 15, 2025

View reviewed changes

amilcarlucas and others added 7 commits August 18, 2025 03:36

feat(motor test): implement frontend

d5b0344

fix(motor test): simplify motor labels gen

fe29b22

refactor(motor_test): methods raise exceptions instead of returning e…

620942a

…rror_messages in tuples. This is more pythonic

fix(motor_test): Correct motor_test default settings values

fd4fdef

feat(motor_test): More work on the motor tests sub-application

80bc4cf

feat(motor test): convert motor test diagrams from SVG into PNG

9b88b85

Required because TKinter can not display .svg files

[pre-commit.ci] auto fixes from pre-commit.com hooks

260b790

for more information, see https://pre-commit.ci

amilcarlucas force-pushed the motor_test_gui branch from f55c184 to 260b790 Compare August 18, 2025 01:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Motor test gui #700

Motor test gui #700

Uh oh!

amilcarlucas commented Aug 15, 2025

Uh oh!

github-actions bot commented Aug 15, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 15, 2025

Uh oh!

Copilot AI Aug 15, 2025

Uh oh!

Copilot AI Aug 15, 2025

Uh oh!

Copilot AI Aug 15, 2025

Uh oh!

Copilot AI Aug 15, 2025

Uh oh!

Copilot AI Aug 15, 2025

Uh oh!

Copilot AI Aug 15, 2025

Uh oh!

Uh oh!

-    convert_svg_to_png(result_height=320)
+    parser = argparse.ArgumentParser(
+        description="Download and/or convert ArduPilot motor diagram SVG files."
+    )
+    parser.add_argument(
+        "--download",
+        action="store_true",
+        help="Download all motor diagram SVG files."
+    )
+    parser.add_argument(
+        "--convert",
+        action="store_true",
+        help="Convert all SVG files to PNG."
+    )
+    parser.add_argument(
+        "--height",
+        type=int,
+        default=320,
+        help="Resulting PNG height in pixels (default: 320)."
+    )
+    args = parser.parse_args()
+    if args.download:
+        download_motor_diagrams()
+    if args.convert:
+        convert_svg_to_png(result_height=args.height)
+    if not args.download and not args.convert:
+        parser.print_help()

	logging_error(_("Found SVG diagram at: %(path)s"), {"path": diagram_path})
	logging_info(_("Found SVG diagram at: %(path)s"), {"path": diagram_path})

	self._test_duration_s = int(duration)
	self._test_duration_s = float(duration)

	return int(self._test_throttle_pct)
	return self._test_throttle_pct

	raise TimeoutError(_("Timeout waiting for parameter %s") % param_name)
	raise TimeoutError(_("Timeout waiting for parameter %s after %d seconds") % (param_name, timeout))

	pytest.raises(Exception, match="Test exception"),
	with (patch.object(MotorTestDataModel, "_update_frame_configuration", side_effect=Exception("Test exception")),
	pytest.raises(Exception, match="Test exception"),

Motor test gui #700

Are you sure you want to change the base?

Motor test gui #700

Uh oh!

Conversation

amilcarlucas commented Aug 15, 2025

Uh oh!

github-actions bot commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 15, 2025 •

edited

Loading