Add LeadingEdgeDevices #1109

ole-alb · 2025-07-22T09:11:57Z

In TiGL 3 we only provide the implementation for TrailingEdgeDevices. This will be extended by LeadingEdgeDevices.

Description

How Has This Been Tested?

Screenshots, that help to understand the changes(if applicable):

Checklist:

Task	Finished	Reviewer Approved
At least one test for the new functionality was added.	yes does not apply	OK
New classes have been added to the Python interface.	yes does not apply	OK
The code is properly documented with doxygen docstrings	yes does not apply	OK
Changes are documented at the top of ChangeLog.md	yes does not apply	OK

codecov · 2025-07-22T09:27:25Z

Codecov Report

❌ Patch coverage is 72.08738% with 230 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.48%. Comparing base (08f4898) to head (72393f9).

Files with missing lines	Patch %	Lines
...control_devices/CCPACSControlSurfaceWingCutOut.cpp	0.00%	82 Missing ⚠️
..._devices/CCPACSControlSurfaceBorderLeadingEdge.cpp	50.00%	40 Missing ⚠️
src/api/tigl.cpp	67.67%	32 Missing ⚠️
src/wing/CCPACSWing.cpp	90.73%	24 Missing ⚠️
src/control_devices/CCPACSLeadingEdgeDevice.cpp	75.00%	21 Missing ⚠️
...c/control_devices/CControlSurfaceBorderBuilder.cpp	77.58%	13 Missing ⚠️
src/control_devices/ControlSurfaceDeviceHelper.cpp	87.03%	7 Missing ⚠️
...devices/CCPACSControlSurfaceBorderTrailingEdge.cpp	63.63%	4 Missing ⚠️
...ices/CCPACSControlSurfaceOuterShapeLeadingEdge.cpp	87.50%	4 Missing ⚠️
src/wing/CCPACSWingCSStructure.cpp	0.00%	2 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1109      +/-   ##
==========================================
- Coverage   72.75%   72.48%   -0.27%     
==========================================
  Files         308      313       +5     
  Lines       26993    27381     +388     
==========================================
+ Hits        19638    19848     +210     
- Misses       7355     7533     +178

Flag	Coverage Δ
unittests	`72.48% <72.08%> (-0.27%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/CCPACSEtaIsoLine.cpp	`71.42% <100.00%> (+2.19%)`	⬆️
src/CCPACSXsiIsoLine.cpp	`25.00% <100.00%> (+10.71%)`	⬆️
...ces/CCPACSControlSurfaceOuterShapeTrailingEdge.cpp	`87.09% <100.00%> (-1.80%)`	⬇️
src/control_devices/CCPACSControlSurfaceSteps.cpp	`100.00% <100.00%> (ø)`
src/control_devices/CCPACSControlSurfaces.cpp	`100.00% <100.00%> (ø)`
src/control_devices/CCPACSTrailingEdgeDevice.cpp	`88.09% <95.83%> (+2.29%)`	⬆️
src/wing/CCPACSWingCSStructure.cpp	`42.62% <0.00%> (-1.45%)`	⬇️
...devices/CCPACSControlSurfaceBorderTrailingEdge.cpp	`50.70% <63.63%> (ø)`
...ices/CCPACSControlSurfaceOuterShapeLeadingEdge.cpp	`87.50% <87.50%> (ø)`
src/control_devices/ControlSurfaceDeviceHelper.cpp	`87.03% <87.03%> (ø)`
... and 6 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…oilers

…es-Spoilers

…oilers

bindings/python_internal/configuration.i

joergbrech

I reviewed in parts. I still have to look at the unit tests and the implementation in src/control_devices.

I do have a few comments so far, see below.

I also have some general comments/questions:

In tigl.cpp there are two places where we iterate over TEDs and LEDs. In the first, we are building a fused shape of the control surface cutout shapes to buil the wing loft with cutouts. In the second place, we are collecting all LED and TED shapes. I see a chance of moving both functionalities into the CCPACSControlSurfaces class, so that the function bodies in tigl.cpp are less daunting. It feels like a better seperation of concerns to me.
Currently, the control surface cutout shapes are first fused individually one-by-one to one big cutout shape, which is then removed from the wing (if I am not mistaken). Building the cutout shape take quite a long time and I wonder if we can improve the performance here by doing n Boolean cuts instead of n Boolean fuses and then one big complicated cut. But we would have to experiment if we actually gain anything from it I suppose. Could also be part of a separate PR.

src/api/tigl.h

src/CCPACSXsiIsoLine.cpp

src/CCPACSEtaIsoLine.cpp

src/wing/CCPACSWing.cpp

joergbrech · 2025-11-05T09:44:52Z

src/wing/CCPACSWing.cpp

+            const CCPACSLeadingEdgeDevices& LeadingEdgeDevices = controlSurfs.GetLeadingEdgeDevices().value();
+
+            for (size_t j = LeadingEdgeDevices.GetLeadingEdgeDevices().size(); j > 0; j--) {
+                CCPACSLeadingEdgeDevice& LeadingEdgeDevice = *LeadingEdgeDevices.GetLeadingEdgeDevices().at(j - 1);


See above. I don't understand why we go from back to front.

src/wing/CCPACSWing.cpp

src/control_devices/CCPACSControlSurfaces.h

src/control_devices/CCPACSControlSurfaces.cpp

joergbrech · 2025-11-21T08:05:59Z

TIGLCreator/src/TIGLCreatorSelectWingAndFlapStatusDialog.h

+    };
+
+    std::map<std::string, DeviceWidgets> _guiMap;
+    std::map<std::string, tigl::CTiglAbstractGeometricComponent*> _deviceMap;


The code works and reflects the previous implementation for TEDs. But I have a suggestion for refactoring.

Problems: We store pointers to CTiglAbstractGeometricComponent in a map here. One conceptual issue is that not all CTiglAbstractGeometricComponents are control devices. Another issue is that we already manage a map that gives us geometric components given a uid.

Solution: At all places where we currently use the _deviceMap, let's use the uid manager instead. We can query if a given uid is of an expected type, e.g. LED or TED and then resolve this object through the UID manager interface.

I made some adjustements based on your suggestion

I think we can do completely without the _deviceMap.

TIGLCreator/src/TIGLCreatorSelectWingAndFlapStatusDialog.cpp

src/control_devices/CCPACSControlSurfaceOuterShapeLeadingEdge.cpp

joergbrech · 2025-11-21T10:37:15Z

src/control_devices/CCPACSControlSurfaceOuterShapeLeadingEdge.cpp

+    }
+}
+
+PNamedShape CCPACSControlSurfaceOuterShapeLeadingEdge::CutoutShape(PNamedShape wingShape, gp_Vec upDir) const


Suggestion for more DRYness.

I believe that this function can be made more generic and I have seen this calculation (ThruSections from inner and outer border) twice in CCPACSControlSurfaceWingCutout.

joergbrech · 2025-11-21T10:42:11Z

src/control_devices/CCPACSControlSurfaceBorderLeadingEdge.cpp

+}
+
+TopoDS_Wire
+CCPACSControlSurfaceBorderLeadingEdge::GetAirfoilWire(CTiglControlSurfaceBorderCoordinateSystem& coords) const


Suggestion for more DRYness

This function really only uses a CCPACSWingProfile. So most of the implemenation can move to a function that is shared in the implementation for CCPACSControlSurfaceBorderLeadingEdge and CCPACSControlSurfaceBorderTrailingEdge.

I'm not sure if that works since there is a the difference in the GetAirfoil_choiceX

src/control_devices/CCPACSControlSurfaceBorderLeadingEdge.cpp

joergbrech · 2025-11-21T10:51:14Z

@ole-alb I finally managed to look at the whole PR. Sorry it took so long. Apart from the comments above, I have two additional remarks:

I noticed that the file CCPACSControlSurfaceWingCutOut.cpp isn't covered by any unit test. Is there a chance we can change that?
We still have to find and fix the one bug with the F25 before merging this.

joergbrech · 2025-12-01T10:00:14Z

TIGLCreator/src/TIGLCreatorSelectWingAndFlapStatusDialog.h

+    struct TED 
+    {
+        tigl::CCPACSTrailingEdgeDevice* device;
+        std::string uid() const { return device->GetUID(); }
+        double control_parameter() const { return device->GetControlParameter(); }
+    };
+
+    struct LED 
+    {
+        tigl::CCPACSLeadingEdgeDevice* device;
+        std::string uid() const { return device->GetUID(); }
+        double control_parameter() const { return device->GetControlParameter(); }
+    };
+
+    using ControlDevice = std::variant<TED, LED>;


Why the extra structs? Why not

using ControlDevice = std::variant<tigl::CCPACSTrailingEdgeDevice*, tigl::CCPACSLeadingEdgeDevice*>;

This could be a one-liner header file in src/conrol_devices, so that you can reuse the variant interface for other shared functionality between LEDs and TEDs, not just here in the TiGLCreator code.

joergbrech · 2025-12-01T10:05:59Z

src/control_devices/CCPACSTrailingEdgeDevice.cpp

        shape = GetWingCutOut()->GetLoft(ComponentSegment(*this).GetWing().GetWingCleanShape(), GetOuterShape(),
                                         GetNormalOfControlSurfaceDevice());


We can delete this, no?

joergbrech · 2025-12-01T10:15:11Z

src/control_devices/ControlSurfaceDeviceHelper.h

This looks fine, I just have two style comments:

Personally, i would remove the _helper postfix in the method names.

Since the class does not have any data and doesn't need an implicit this argument in any of its functions, it could be made a namespace and all methods become free functions.

ole-alb added 4 commits July 21, 2025 15:38

run code generator for LED

028d7c5

update code generator files

7bf4afa

add LED custom classes

b39ee07

add LED

482f25f

ole-alb added 3 commits August 1, 2025 10:48

LED and TED are Realtivelypostionedcomponents

0d1ae15

Merge remote-tracking branch 'origin' into 1101-LeadingEdgeDevices-Sp…

968ca53

…oilers

fix include

a978528

ole-alb changed the base branch from tigl-3-4-1 to main September 11, 2025 14:39

ole-alb and others added 6 commits September 12, 2025 12:51

add LEDs to tiglcreator

9703b2b

switch base class back to CTiglAbstractGeometricComponent

ca70905

Merge remote-tracking branch 'origin/main' into 1101-LeadingEdgeDevic…

4e5a4f5

…es-Spoilers

fix LED cutout and shape

7cf9060

Merge branch 'main' into 1101-LeadingEdgeDevices-Spoilers

6289dba

cleanup

d09f36c

ole-alb changed the title ~~Add LeadingEdgeDevices and Spoilers~~ Add LeadingEdgeDevices Sep 19, 2025

ole-alb and others added 8 commits September 19, 2025 14:19

add LED to api

a5ce33b

cleanup

c20ea20

add LEDs to testfile cpacs

19efba6

revert api changes

46d037e

add new LED and TED functions to api

68a9c50

update tests for new api functions

a6260b3

add new testfile with LEDs

2979cbe

Merge branch 'main' into 1101-LeadingEdgeDevices-Spoilers

410e3e4

ole-alb marked this pull request as ready for review September 25, 2025 09:23

add LED to python bindings

aa51b45

ole-alb requested a review from joergbrech September 26, 2025 08:55

add changelog

59671e7

joergbrech mentioned this pull request Oct 13, 2025

Always display the control devices with the wing #1233

Merged

Merge remote-tracking branch 'origin/main' into 1101-LeadingEdgeDevic…

1d0b6d9

…es-Spoilers

ole-alb added 2 commits October 24, 2025 10:48

Merge remote-tracking branch 'origin' into 1101-LeadingEdgeDevices-Sp…

9f931a7

…oilers

always display leds

b8be102

joergbrech reviewed Nov 5, 2025

View reviewed changes

bindings/python_internal/configuration.i Show resolved Hide resolved

joergbrech requested changes Nov 5, 2025

View reviewed changes

ole-alb added 7 commits November 6, 2025 11:14

apply review

8e388a9

create unified api functions for control surfaces

ea42828

reverse for loops

a3c7bfc

fix helper function

1bd3142

split tiglGetControlSurfaceUID into functions for ted and led

e51eb5e

test led api in python

1dbfe49

add further python tests

c12917f

joergbrech reviewed Nov 7, 2025

View reviewed changes

src/wing/CCPACSWing.cpp Outdated Show resolved Hide resolved

joergbrech requested changes Nov 21, 2025

View reviewed changes

ole-alb and others added 8 commits November 26, 2025 11:03

apply review

2ee688d

add CCPACSControlSurfaceSteps

1ea5426

add ControlSurfaceDeviceHelper

947af71

Merge branch 'main' into 1101-LeadingEdgeDevices-Spoilers

51d9ea8

fix small bug

39dd54b

fix helper

240b233

_uid -> uid

1c58c25

add CCPACSControlSurfaceSteps to python bindings

72393f9

joergbrech reviewed Dec 1, 2025

View reviewed changes

		shape = GetWingCutOut()->GetLoft(ComponentSegment(*this).GetWing().GetWingCleanShape(), GetOuterShape(),
		GetNormalOfControlSurfaceDevice());

Add LeadingEdgeDevices #1109

Are you sure you want to change the base?

Add LeadingEdgeDevices #1109

Conversation

ole-alb commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Screenshots, that help to understand the changes(if applicable):

Checklist:

Uh oh!

codecov bot commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

joergbrech left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joergbrech commented Nov 21, 2025 • edited by ole-alb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ole-alb commented Jul 22, 2025 •

edited

Loading

codecov bot commented Jul 22, 2025 •

edited

Loading

joergbrech commented Nov 21, 2025 •

edited by ole-alb

Loading