DRY refactoring, restore APIs, and full magnetic assembly tests by tinix84 · Pull Request #12 · OpenMagnetics/MVB

tinix84 · 2026-02-06T07:54:09Z

Summary

DRY refactoring: Eliminate ~350 lines of duplicated code across CadQuery and FreeCAD engines (shared flatten_dimensions, BuilderBase, shape_configs, default get_shape_extras, extracted SVG helper)
Fix test failures: Resolve pre-existing test_1 (missing get_piece_technical_drawing) and test_4 (UR S-dimension OCC boolean failure) issues
Restore lost APIs: Re-add get_magnetic_assembly, get_bobbin, get_winding to Builder facade and CadQueryBuilder (lost during full-magnetic merge)
Restore test files: Bring back test_assembly.py, test_bobbin.py, test_winding.py with all original tests
Add Ut shape: Implement UT core family in CadQuery builder
SVG technical drawings: Implement get_piece_technical_drawing and get_core_gapping_technical_drawing using CadQuery SVG export (fix hex-to-RGB color conversion)
Full magnetic tests: Add ETD49 (round wire, 6 turns) and PQ4040 (foil wire, 6 turns) MAS test data and validation tests

Test plan

All 14 test_builder tests pass
All 9 test_assembly tests pass (7 OK + 2 skipped)
All 4 test_bobbin tests pass
All 7 test_winding tests pass (6 OK + 1 skipped)
ETD49 round wire full magnetic generates 9 solids (2 core + 1 bobbin + 6 turns)
PQ4040 foil wire full magnetic generates 9 solids (2 core + 1 bobbin + 6 turns)
SVG technical drawings render correctly in Chrome/Inkscape

🤖 Generated with Claude Code

Implement dual-mode winding generation that uses exact MAS turnsDescription coordinates when available, falling back to calculated positions otherwise. Changes: - Add TurnDescription dataclass with from_dict() factory method - Add create_turn_from_description() for single turn from MAS coordinates - Add get_winding_from_mas() to build winding from turnsDescription array - Update get_winding() to check for MAS data first, then fall back - Update AssemblyBuilder to pass complete coil data with turnsDescription - Add comprehensive tests for MAS mode and fallback behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

The full-magnetic branch uses a different API (get_magnetic instead of get_winding/get_bobbin/get_magnetic_assembly). Remove old test files as they are replaced by test_magnetic.py with comprehensive tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Extract shared flatten_dimensions() and BuilderBase into utils.py, create shape_configs.py for shared dimension dicts, make C inherit from U, add default get_shape_extras in CadQuery IPiece, extract _rotate_piece_z_180, _create_rectangular_sketch, and _create_dimension_svg helpers in FreeCAD builder, and fix broken imports in magnetic_builder.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add get_piece_technical_drawing stub to CadQuery IPiece (method only existed in FreeCAD engine). Fix UR shape S-dimension boolean failure by extending the right keyway hole slightly past the winding column surface to avoid tangent-surface OCC kernel errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Add get_magnetic_assembly, get_bobbin, get_winding to Builder facade - Add get_magnetic_assembly, get_bobbin, get_winding to CadQueryBuilder - Restore IBobbin/StandardBobbin and IWinding/RoundWireWinding inner classes (lost during full-magnetic merge in 1b925c7) - Add Ut shape class to CadQuery builder for UT core family - Implement get_piece_technical_drawing with CadQuery SVG export - Implement get_core_gapping_technical_drawing with CadQuery SVG export - Fix SVG strokeColor: convert hex string to RGB tuple for CadQuery exporter - Rename internal get_bobbin(BobbinProcessedDescription) to _build_bobbin_geometry - Restore test_assembly.py, test_bobbin.py, test_winding.py from pre-merge - Add ETD49 and PQ4040 MAS test data files - Add TestFullMagnetic tests for ETD49 (round wire) and PQ4040 (foil wire) All 34 tests pass (31 passed, 3 skipped). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR performs a substantial DRY refactoring to eliminate ~350 lines of duplicated code across FreeCAD and CadQuery engines, while also fixing test failures, restoring previously lost APIs, and adding comprehensive test coverage for magnetic assemblies.

Changes:

Extracts shared code into utils.py (flatten_dimensions, BuilderBase) and shape_configs.py (dimension/subtype configurations)
Restores critical APIs to Builder facade: get_magnetic_assembly, get_bobbin, get_winding
Implements UT core shape family and SVG technical drawing generation with hex-to-RGB color conversion
Adds comprehensive test suites for assembly (537 lines), bobbin (132 lines), and winding (216 lines) components
Switches test_builder.py from FreeCAD to CadQuery engine and adds test_3 and test_4 for UR shape edge cases
Adds full magnetic assembly tests for ETD49 (round wire) and PQ4040 (foil wire) with geometry validation

Reviewed changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
src/OpenMagneticsVirtualBuilder/utils.py	Adds shared `flatten_dimensions` function and `BuilderBase` class for factory/families logic
src/OpenMagneticsVirtualBuilder/shape_configs.py	New file with shared P, RM, UR, U shape dimension/subtype configurations
src/OpenMagneticsVirtualBuilder/freecad_builder.py	Inherits from BuilderBase, uses shared configs, extracts `_create_dimension_svg` and `_create_rectangular_sketch` helpers
src/OpenMagneticsVirtualBuilder/cadquery_builder.py	Implements `get_magnetic_assembly`, `get_bobbin`, `get_winding`, `get_piece_technical_drawing`, adds UT shape, fixes UR S-dimension boolean failure with epsilon offset
src/OpenMagneticsVirtualBuilder/builder.py	Restores API facade methods for magnetic_assembly, bobbin, and winding
src/OpenMagneticsVirtualBuilder/magnetic_builder.py	Removes unused imports (convert_axis_toroidal, build_magnetic_from_mas)
tests/test_assembly.py	New comprehensive assembly test suite (9 tests including MAS-based full assemblies)
tests/test_bobbin.py	New bobbin test suite (4 tests covering standard bobbins with/without pins)
tests/test_winding.py	New winding test suite (7 tests covering round wire, bulk, and MAS turnsDescription modes)
tests/test_magnetic.py	Adds TestFullMagnetic class with ETD49 and PQ4040 real-world assembly validation
tests/test_builder.py	Switches engine from FreeCAD to CadQuery, adds test_3 (UR type 2) and test_4 (UR 35/27.5/13 with S-dimension fix)
pyproject.toml	Bumps version from 0.6.11 to 0.6.12

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T07:59:18Z

src/OpenMagneticsVirtualBuilder/freecad_builder.py

+                    if len(gaps_per_column) > 1 and gap_index < len(gaps_per_column) - 1:
+                        next_gap = gaps_per_column[gap_index + 1]
+                        chunk_size = (-gap['length'] / 2 - gap['coordinates'][1]) - (next_gap['length'] / 2 - next_gap['coordinates'][1])
+                        dimension_label = f"{round(chunk_size * 1000, 2)}"
+                        if chunk_size * 1000 * scale > 70:
+                            dimension_label += " mm"
+                        svgFile_data += create_dimension(ending_coordinates=[((gap['coordinates'][0] + gap['sectionDimensions'][0] / 2) * 1000 + center_offset) * scale, (-gap['length'] / 2 - gap['coordinates'][1]) * scale * 1000],
+                                                         starting_coordinates=[((gap['coordinates'][0] + gap['sectionDimensions'][0] / 2) * 1000 + center_offset) * scale, (next_gap['length'] / 2 - next_gap['coordinates'][1]) * scale * 1000],
+                                                         dimension_type="DistanceY",
+                                                         dimension_label=dimension_label,
+                                                         label_offset=min(base_width / 2, horizontal_offset),
+                                                         label_alignment=(-gap['coordinates'][1] - gap['length'] / 2 - chunk_size / 2) * scale * 1000)


Duplicate code block detected: Lines 485-496 are identical to lines 498-509. This appears to be an accidental duplication where the same chunk size calculation and dimension creation for gaps_per_column is executed twice. The second occurrence (lines 498-509) should be removed.

Copilot · 2026-02-06T07:59:18Z

tests/test_magnetic.py

+    def test_etd49_round_wire_5_turns(self):
+        """Test ETD49 core with 6 round wire turns and bobbin."""
+        step_path, stl_path, solid_info = self._run_test(
+            'ETD49_N87_10uH_5T.json', validate_geometry=True
+        )
+
+        print(f"\n=== ETD49 Geometry ===")
+        print(f"Total solids: {len(solid_info)}")
+        for s in solid_info:
+            print(f"  Solid {s['index']}: vol={s['volume']:.1f}")
+
+        # 2 core halves + 1 bobbin + 6 turns = 9
+        assert len(solid_info) == 9, f"Expected 9 solids, got {len(solid_info)}"


Inconsistency between test name and expectations: The test function is named test_etd49_round_wire_5_turns and loads ETD49_N87_10uH_5T.json (both suggesting 5 turns), but the docstring says "6 round wire turns", the inline comment says "6 turns", and the assertion expects 9 solids (2 core halves + 1 bobbin + 6 turns). Either rename the test to test_etd49_round_wire_6_turns or update the docstring, comment, and assertion to reflect 5 turns and expect 8 solids total.

Copilot · 2026-02-06T07:59:18Z

src/OpenMagneticsVirtualBuilder/cadquery_builder.py

+                wall_thickness = processed.get("wallThickness", dims.get("wallThickness", 0.0005))
+                column_shape = processed.get("columnShape", "rectangular")
+                column_width = processed.get("columnWidth", 0)
+                column_depth = processed.get("columnDepth", column_width)


Variable column_depth is not used.

Copilot · 2026-02-06T07:59:19Z

src/OpenMagneticsVirtualBuilder/cadquery_builder.py

+                ww_height = winding_window.get("height", 0)
+                column_shape = winding_window.get("columnShape", "rectangular")
+                column_width = winding_window.get("columnWidth", 0)
+                column_depth = winding_window.get("columnDepth", column_width)


Variable column_depth is not used.

Copilot · 2026-02-06T07:59:19Z

src/OpenMagneticsVirtualBuilder/cadquery_builder.py

+        def get_bulk_winding(self, data, bobbin_dims):
+            wire_diameter = data.get("wireDiameter", 0.0005)
+            insulation = data.get("insulationThickness", 0.00005)
+            num_turns = data.get("numberOfTurns", 1)


Variable num_turns is not used.

Suggested change

num_turns = data.get("numberOfTurns", 1)

Copilot · 2026-02-06T07:59:21Z

tests/test_winding.py

@@ -0,0 +1,216 @@
+import unittest
+import os
+import glob


Import of 'glob' is not used.

Suggested change

import glob

Copilot · 2026-02-06T07:59:21Z

tests/test_winding.py

+import glob
+import json
+
+import context  # noqa: F401


Import of 'context' is not used.

Copilot · 2026-02-06T07:59:22Z

src/OpenMagneticsVirtualBuilder/cadquery_builder.py

+            else:
+                return pieces
+
+        except:  # noqa: E722


Except block directly handles BaseException.

Suggested change

except: # noqa: E722

except Exception: # noqa: E722

Copilot · 2026-02-06T07:59:22Z

tests/test_assembly.py

+                    data = json.loads(ndjson_line)
+                    if data["name"] == "ETD 49/25/16":
+                        return data
+        except FileNotFoundError:


'except' clause does nothing but pass and there is no explanatory comment.

Copilot · 2026-02-06T07:59:22Z

tests/test_assembly.py

+                    data = json.loads(ndjson_line)
+                    if data["name"] == "PQ 40/40":
+                        return data
+        except FileNotFoundError:


'except' clause does nothing but pass and there is no explanatory comment.

- Remove duplicate code block in freecad_builder.py gaps_per_column - Remove unused column_depth in StandardBobbin.get_bobbin_body - Remove unused num_turns in RoundWireWinding.get_bulk_winding - Remove unused dimensions assignment in freecad_builder toroidal branch - Remove dead 't'/'ut' family branches in test_builder.py (already excluded by filter) - Replace unnecessary core= assignments with direct calls in test_builder.py - Remove unused glob imports from test_bobbin.py and test_winding.py - Replace bare pass in except clauses with explicit return in test_assembly.py - Fix test name: test_etd49_round_wire_5_turns -> test_etd49_round_wire_6_turns Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Set up ruff linter and vulture dead code detector as pre-commit hooks. Fixed 267 auto-fixable issues and 40 manual fixes including: trailing whitespace in docstrings, assert False -> raise AssertionError, open() without context manager, unused variables, and capitalized env vars. Applied ruff format to all source and test files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Implement multi-format 2D technical drawing generation using OCC hidden-line removal (HLRBRep_Algo) for correct visible-edge projection. DXF uses CadQuery's DxfDocument, FCMacro uses CadQuery edge API with degenerate edge filter to fix FreeCAD "Both points are equal" crash. Dimensions removed from all formats (kept as unused code for future re-enablement). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

AlfVII and others added 8 commits May 22, 2025 21:57

Merge

85c4ca5

Fixed some issues with URs

b482312

Merge full-magnetic branch - prioritizing origin implementation

598ede6

Copilot AI review requested due to automatic review settings February 6, 2026 07:54

Copilot started reviewing on behalf of tinix84 February 6, 2026 07:54 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

tinix84 and others added 2 commits February 6, 2026 10:11

AlfVII approved these changes Feb 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRY refactoring, restore APIs, and full magnetic assembly tests#12

DRY refactoring, restore APIs, and full magnetic assembly tests#12
tinix84 wants to merge 11 commits intoOpenMagnetics:full-magneticfrom
tinix84:magnetic_assembly

tinix84 commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

tinix84 commented Feb 6, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants