Gcode: support 6 axis machine (work line ncviewer.com) by ABSitf · Pull Request #1403 · MeshInspector/MeshLib

ABSitf · 2023-07-10T07:45:21Z

No description provided.

…o i2198_settings_6_axis_machine

Grantim · 2023-07-10T08:05:53Z

source/MRMesh/MRGcodeProcessor.h

+    // settings
+    enum class RotationParameterName
+    {
+        None = -1,
+        A,
+        B,
+        C,
+        Count
+    };
+    MRMESH_API void setRotationParams( RotationParameterName paramName, const Vector3f& rotationAxis );
+    MRMESH_API Vector3f getRotationParams( RotationParameterName paramName );
+    MRMESH_API bool setRotationOrder( std::array<RotationParameterName, 3> rotationAxisOrder );
+    MRMESH_API std::array<RotationParameterName, 3> getRotationOrder();


mb make single structure with gcode axis name to real axis map, and order, and one getter setter for it

it's possible, but i don't see serious advantages

I think it will be just more friendly interface

Grantim · 2023-07-10T08:09:08Z

source/MRMesh/MRGcodeProcessor.h

do we need translation order too? mb some devices has moving axis (that means that translation is applied before rotation)

in current version it has no effect (will added in next PR)

Grantim · 2023-07-10T08:10:06Z

source/MRMesh/MRGcodeProcessor.cpp

+GcodeProcessor::GcodeProcessor()
+{
+    rotationAxes_ = { Vector3f::minusX(), Vector3f::minusY(), Vector3f::plusZ() };
+    setRotationOrder( { RotationParameterName::A, RotationParameterName::B, RotationParameterName::C } );
+}
+


why do in constructor, mb just initialize it in class body?

Grantim · 2023-07-10T08:11:52Z

source/MRMesh/MRGcodeProcessor.cpp

+    const bool validParamName = intParamName >= int( RotationParameterName::A ) && intParamName < int( RotationParameterName::Count );
+    assert( validParamName );
+    if ( !validParamName )
+        return;


looks like excessive checks

no, not all RotationParameterName have axis (RotationParameterName::None or Count)

Grantim · 2023-07-10T08:12:19Z

source/MRMesh/MRGcodeProcessor.cpp

+    const int intParamName = int( paramName );
+    const bool validParamName = intParamName >= int( RotationParameterName::A ) && intParamName < int( RotationParameterName::Count );
+    assert( validParamName );


looks like escessive test

assert removed. range check saved (same reason)

Grantim · 2023-07-10T08:13:51Z

source/MRMesh/MRGcodeProcessor.h

+    MRMESH_API void setRotationParams( RotationParameterName paramName, const Vector3f& rotationAxis );
+    MRMESH_API Vector3f getRotationParams( RotationParameterName paramName );
+    MRMESH_API bool setRotationOrder( std::array<RotationParameterName, 3> rotationAxisOrder );
+    MRMESH_API std::array<RotationParameterName, 3> getRotationOrder();


return const ref, and make the function const

Grantim · 2023-07-10T08:14:21Z

source/MRMesh/MRGcodeProcessor.h

    MRMESH_API MoveAction processLine( const std::string_view& line );

+    // settings
+    enum class RotationParameterName


mb RotationAxisName

Grantim · 2023-07-10T08:16:19Z

source/MRMesh/MRGcodeProcessor.h

+    };
+    MRMESH_API void setRotationParams( RotationParameterName paramName, const Vector3f& rotationAxis );
+    MRMESH_API Vector3f getRotationParams( RotationParameterName paramName );
+    MRMESH_API bool setRotationOrder( std::array<RotationParameterName, 3> rotationAxisOrder );


mb make alias?

using RotationAxisMap = std::array<RotationParameterName, size_t(RotationParameterName::Count)>;

pass non-trivial args my reference

const RotationAxisMap&

Grantim · 2023-07-10T08:18:33Z

source/MRMesh/MRGcodeProcessor.cpp

+        const int parameterIndex = int( rotationAxisOrder[i] );
+        if ( parameterIndex < int( RotationParameterName::None ) || parameterIndex >= int( RotationParameterName::Count ) )
+        {
+            validInput = false;
+            break;
+        }


excessive checks

Grantim · 2023-07-11T15:58:41Z

source/MRMesh/MRGcodeProcessor.h

+    // settings
+    enum class RotationAxisName
+    {
+        None = -1,


do we need None?

Grantim · 2023-07-11T15:59:54Z

source/MRMesh/MRGcodeProcessor.cpp

+        if ( axisIndex < 0 || axisIndex > 2 )
+            continue;


Grantim · 2023-07-11T16:00:27Z

source/MRMesh/MRGcodeProcessor.cpp

+        if ( axisIndex < 0 || axisIndex > 2 )
+            continue;


ABSitf added 3 commits July 7, 2023 11:19

added settings rotation axis and rotation order for 6 axis machine

c0e960a

Merge branch 'master' of https://github.com/MeshInspector/MeshLib int…

11aa062

…o i2198_settings_6_axis_machine

remove unused code�added missing method for settings

3ab61f7

Grantim requested changes Jul 10, 2023

View reviewed changes

fix

d5396d1

Grantim reviewed Jul 11, 2023

View reviewed changes

fix�small rework (removed RotationAxisName::None)

974bc2d

Grantim approved these changes Jul 12, 2023

View reviewed changes

ABSitf merged commit 1bba226 into master Jul 12, 2023

ABSitf deleted the i2198_settings_6_axis_machine branch July 12, 2023 09:04

Conversation

ABSitf commented Jul 10, 2023

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!

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!

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!

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!

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!

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!

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!

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!

2 participants