Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added API function tiglFuselageGetSectionCenter #270

Merged
merged 4 commits into from Jul 7, 2017

Conversation

hereToCreate
Copy link
Contributor

A function for computing fuselage section center is committed.

Fixes issue #260

A function for computing fuselage section center is committed.

Fixes issue DLR-SC#260
Copy link
Collaborator

@rainman110 rainman110 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check my comments and make the required changes.

src/api/tigl.cpp Outdated
TopoDS_Shape curve = segment.getWireOnLoft(eta);

// get linear properties of the ISO curve
GProp_GProps LProps;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put the next 5 lines into a separate function in tiglcommonfunctions.

EXPECT_NEAR(pointZ, 0, 1e-2);

ASSERT_EQ(TIGL_UID_ERROR, tiglFuselageGetSectionCenter(tiglHandle, "segmentD150_Fuselaggg_1Segment2ID", eta, &pointX, &pointY, &pointZ));
ASSERT_EQ(TIGL_NULL_POINTER, tiglFuselageGetSectionCenter(tiglHandle, "segmentD150_Fuselage_1Segment2ID", eta, NULL, &pointY, &pointZ));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add more tests for NULL pointers and a test for an invalid handle.

src/api/tigl.cpp Outdated
@@ -62,6 +62,8 @@
#include "gp_Pnt.hxx"
#include "TopoDS_Shape.hxx"
#include "TopoDS_Edge.hxx"
#include "GProp_GProps.hxx"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these includes, after putting the code into a separate function.

Some lines in tiglFuselageGetSectionCenter are bundled in a helper
function. The test for this method is enhanced.

Fixes issue DLR-SC#260
Copy link
Collaborator

@rainman110 rainman110 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks already pretty good. Well done!

Please have a look to my comments. There are some tests missing. They probably require you to add the code in tigl.cpp, but maybe also not. Write the tests first, then the code to fix the tests if necessary.

ASSERT_EQ(TIGL_NULL_POINTER, tiglFuselageGetSectionCenter(tiglHandle, "segmentD150_Fuselage_1Segment2ID", eta, &pointX, NULL, NULL));
ASSERT_EQ(TIGL_NULL_POINTER, tiglFuselageGetSectionCenter(tiglHandle, "segmentD150_Fuselage_1Segment2ID", eta, NULL, NULL, NULL));

ASSERT_EQ(TIGL_NOT_FOUND, tiglFuselageGetSectionCenter(-1, "segmentD150_Fuselage_1Segment2ID", eta, &pointX, &pointY, &pointZ));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add tests for the following use cases:

  • fuselageSegmentUID is NULL -> TIGL_NULL_POINTER
  • eta < 0 - >TIGL_MATH_ERROR
  • eta > 1 - >TIGL_MATH_ERROR

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hereToCreate Could you also add one test for the second fuselage segment? Just to make sure, that the segment transformation works as expected.

src/api/tigl.h Outdated
@@ -1523,6 +1523,27 @@ TIGL_COMMON_EXPORT TiglReturnCode tiglGetFuselageCount(TiglCPACSConfigurationHan
TIGL_COMMON_EXPORT TiglReturnCode tiglFuselageGetSegmentCount(TiglCPACSConfigurationHandle cpacsHandle,
int fuselageIndex,
int* segmentCountPtr);
/**
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a blank line between the previous function and the comment block.

Merlin Pelz added 2 commits July 7, 2017 10:03
The test for tiglFuselageGetSectionCenter tests another fuselage
segment now and more error codes are returned in case of the
corresponding errors and the method itself.

Fixes issue DLR-SC#260
Added Error LOGs in the function tiglFuselageGetSectionCenter.

Fixes issue DLR-SC#260
@rainman110 rainman110 merged commit 004fbf8 into DLR-SC:ms/fuselage_functions Jul 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants