Skip to content

Conversation

@wawanbreton
Copy link
Contributor

CURA-12509

Contains #34

Chris Moore and others added 4 commits March 19, 2025 10:28
Retract and Restart are also bead mode tags, so we cannot add our custom bead mode tags on top of these tags.  (There was a firmware bug that was unintentionally blocking all tags from taking effect so we did not previously see the effects of this bug.)
@github-actions
Copy link

github-actions bot commented Mar 28, 2025

Test Results

1 tests  ±0   1 ✅ ±0   0s ⏱️ ±0s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 2db488f. ± Comparison against base commit 334b4d5.

♻️ This comment has been updated with latest results.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

Pause, // Command to allow for user defined pause.
};

#define MB_BEAD_MODE_TAG(TAG_NAME) \

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro MB_BEAD_MODE_TAG used; consider a constexpr template function

TAG_NAME##_0, \
TAG_NAME##_1

enum class Tag

Choose a reason for hiding this comment

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

⚠️ performance-enum-size ⚠️
enum Tag uses a larger base type (int, size: 4 bytes) than necessary for its value set, consider using std::uint8_t (1 byte) as the base type to reduce its size

{ CommandType::WaitForTemperature, "wait_for_temperature" },
{ CommandType::Pause, "pause" } })

#define MB_JTP_TAG(TAG_NAME, TAG_STR) \

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro MB_JTP_TAG used; consider a constexpr template function

#define MB_JTP_TAG(TAG_NAME, TAG_STR) \
{ Tag::TAG_NAME, #TAG_STR }

#define MB_BEAD_MODE_TAG_DEF(TAG_NAME, TAG_STR) \

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro MB_BEAD_MODE_TAG_DEF used; consider a constexpr template function

MB_JTP_TAG(TAG_NAME##_1, TAG_STR##_1)

NLOHMANN_JSON_SERIALIZE_ENUM(
Tag,

Choose a reason for hiding this comment

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

⚠️ misc-include-cleaner ⚠️
no header providing "dulcificum::botcmd::Tag" is directly included

proto_path.emplace_back(move);
return;
}
else

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
else

Comment on lines 280 to 285
{
// NOTE: A move may only have a single bead mode tag
move->tags.emplace_back(botcmd::Tag::TravelMove);
proto_path.emplace_back(move);
return;
}

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
{
// NOTE: A move may only have a single bead mode tag
move->tags.emplace_back(botcmd::Tag::TravelMove);
proto_path.emplace_back(move);
return;
}
// NOTE: A move may only have a single bead mode tag
move->tags.emplace_back(botcmd::Tag::TravelMove);
proto_path.emplace_back(move);
return;

}
}
else if (state.feature_type == "SKIN")
{

Choose a reason for hiding this comment

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

⚠️ bugprone-branch-clone ⚠️
repeated branch body in conditional chain

@Grumpy-ss
Copy link

Grumpy-ss commented Apr 2, 2025

Since each case is isolated and only a single tag is added, either a map or a switch case is easier to read and debug. Performance wise the map is overkill but just to outline the proposed code compartments:

    if (state.is_retracted)
    {
        move->tags.emplace_back(delta_e > 0 ? botcmd::Tag::Restart : botcmd::Tag::TravelMove);
        state.is_retracted = delta_e <= 0;
        proto_path.emplace_back(move);
        return;
    }
    if (delta_e < 0)
    {
        move->tags.emplace_back(botcmd::Tag::Retract);
        state.is_retracted = true;
    }
    else if (delta_e == 0)
    {
        move->tags.emplace_back(botcmd::Tag::TravelMove);
    }
    else
    {
        static const std::unordered_map<std::string, std::pair<botcmd::Tag, botcmd::Tag>> tag_map = {
            {"WALL-OUTER", {botcmd::Tag::WallOuter_0, botcmd::Tag::WallOuter_1}},
            {"WALL-INNER", {botcmd::Tag::WallInner_0, botcmd::Tag::WallInner_1}},
            {"SKIN", {botcmd::Tag::TopSurface_0, botcmd::Tag::TopSurface_1}},
            {"TOP-SURFACE", {botcmd::Tag::TopSurface_0, botcmd::Tag::TopSurface_1}},
            {"PRIME-TOWER", {botcmd::Tag::PrimeTower_0, botcmd::Tag::PrimeTower_1}},
            {"FILL", {botcmd::Tag::Fill_0, botcmd::Tag::Fill_1}},
            {"Purge", {botcmd::Tag::Raft, botcmd::Tag::Raft}},
            {"SUPPORT", {botcmd::Tag::Support_0, botcmd::Tag::Support_1}},
            {"SUPPORT-INTERFACE", {botcmd::Tag::SupportInterface_0, botcmd::Tag::SupportInterface_1}},
            {"SKIRT", {botcmd::Tag::Skirt_0, botcmd::Tag::Skirt_1}}
        };

        auto it = tag_map.find(state.feature_type);
        if (it != tag_map.end())
        {
            move->tags.emplace_back(state.active_tool == 0 ? it->second.first : it->second.second);
        }
    }

    proto_path.emplace_back(move);

@HellAholic HellAholic merged commit 0eb3c58 into main May 20, 2025
12 checks passed
@HellAholic HellAholic deleted the CURA-12509_restore_Method_HS_printers branch May 20, 2025 17:55
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.

4 participants