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.

Sign up for GitHub

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

Jump to bottom

Minor cleanup #2630

Closed
wants to merge 12 commits into from
Closed

Minor cleanup #2630

wants to merge 12 commits into from

Conversation

digant73
Copy link
Contributor

@digant73 digant73 commented Nov 10, 2022 •
edited
Loading

Minor cleanup on top of last merged PRs:

  • added comments

  • fixed indentation

  • code reduction

  • some common changes made, by a merged PR, only in parseAck.c applied also to interfaceCmd.c

  • As it was already reported by the following comment in parseAck.c:

      // parse and store M420 V1 T1, mesh data (e.g. from Mesh Editor menu)
  //
  // IMPORTANT: It must be placed before the following keywords:
  //            1) echo:Bed Leveling
  //            2) mesh. Z offset:
  //

    all the M420 related keywords HAVE to be maintained close to each other (performance+code readibility) (this is a general approach used on both parseAck.c and interfaceCmd.c used for any related keywords).

  • As it was before a merged PR, this PR is restoring a minor code optimization (less bytes) for some param parsing such as:

        if (ack_starts_with("M201")) param = P_MAX_ACCELERATION;
    if (ack_starts_with("M203")) param = P_MAX_FEED_RATE;

    instead of:

        if (ack_starts_with("M201")) param = P_MAX_ACCELERATION;
    else if (ack_starts_with("M203")) param = P_MAX_FEED_RATE;

    IT WAS NOT a bug the missing else. Obviously that kind of code reduction was applied to params not requiring performance optimization (not executed during a print).

PR STATE: ready for merge

@kisslorand
Copy link
Contributor

kisslorand commented Nov 10, 2022 •
edited
Loading

This PR increases RAM and Flash without any benefit (other than avoiding a casting which is not critical).

Master:
RAM: [== ] 21.6% (used 21200 bytes from 98304 bytes)
Flash: [======== ] 84.7% (used 194396 bytes from 229376 bytes)

This PR:
RAM: [== ] 21.6% (used 21204 bytes from 98304 bytes)
Flash: [======== ] 84.8% (used 194404 bytes from 229376 bytes)

Also it slows down "parseACK()" (a function that's heavily used during print) without any functional benefit.

kisslorand
kisslorand reviewed Nov 26, 2022
View reviewed changes
TFT/src/User/API/interfaceCmd.c
@@ -622,17 +622,22 @@ void sendQueueCmd(void)
}
break;

case 25: // M25
case 25: // M25
case 125: // M125
Copy link
Contributor

Choose a reason for hiding this comment

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

Idea stolen from PR #2603

kisslorand
kisslorand reviewed Nov 26, 2022
View reviewed changes
TFT/src/User/API/interfaceCmd.c
PARAMETER_NAME param = P_FWRETRACT;

if (cmd_value() == 208) param = P_FWRECOVER; // P_FWRECOVER
PARAMETER_NAME param = (cmd_value() == 207) ? P_FWRETRACT : P_FWRECOVER;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy & Paste from PR #2607

kisslorand
kisslorand reviewed Nov 26, 2022
View reviewed changes
TFT/src/User/API/interfaceCmd.c
PARAMETER_NAME param = P_HOTEND_PID;

if (cmd_value() == 304) param = P_BED_PID; // P_BED_PID
PARAMETER_NAME param = (cmd_value() == 301) ? P_HOTEND_PID : P_BED_PID;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy & Paste from PR #2607

kisslorand
kisslorand reviewed Nov 26, 2022
View reviewed changes
TFT/src/User/API/interfaceCmd.c
PARAMETER_NAME param = P_DELTA_TOWER_ANGLE;

if (cmd_value() == 666) param = P_DELTA_ENDSTOP; // P_DELTA_ENDSTOP
PARAMETER_NAME param = (cmd_value() == 665) ? P_DELTA_TOWER_ANGLE : P_DELTA_ENDSTOP;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy & Paste from PR #2607

kisslorand
kisslorand reviewed Nov 26, 2022
View reviewed changes
TFT/src/User/API/interfaceCmd.c
@@ -1309,7 +1261,7 @@ void sendQueueCmd(void)
uint8_t v = cmd_value();

if (v == 1 || v == 2)
setParameter(P_ABL_STATE, 0, v % 2); // value will be 1 if v == 1, 0 if v == 2
setParameter(P_ABL_STATE, 0, v & 1U); // value will be 1 if v == 1, 0 if v == 2
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy & Paste from PR #2603

kisslorand
kisslorand reviewed Nov 26, 2022
View reviewed changes
TFT/src/User/API/parseACK.c
@@ -754,8 +778,7 @@ void parseACK(void)
levelingSetProbedPoint(-1, -1, ack_value()); // save probed Z value
sprintf(tmpMsg, "%s\nStandard Deviation: %0.5f", (char *)getDialogMsgStr(), ack_value());

setDialogText((uint8_t *)"Repeatability Test", (uint8_t *)tmpMsg, LABEL_CONFIRM, LABEL_NULL);
showDialog(DIALOG_TYPE_INFO, NULL, NULL, NULL);
popupReminder(DIALOG_TYPE_INFO, (uint8_t *)"Repeatability Test", (uint8_t *)tmpMsg);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy & Paste from PR #2603

kisslorand
kisslorand reviewed Nov 26, 2022
View reviewed changes
TFT/src/User/Menu/Print.c
@@ -164,7 +164,7 @@ bool printPageItemSelected(uint16_t index)
char temp_info[FILE_NUM + 50];
sprintf(temp_info, (char *)textSelect(LABEL_START_PRINT), (uint8_t *)(filename)); // display short or long filename

// confirm file selction
// confirm file selection
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy & paste from PR #2603

@digant73 digant73 closed this Jan 15, 2023
kisslorand
kisslorand reviewed Jan 22, 2023
View reviewed changes
TFT/src/User/Menu/Popup.h
setDialogText(_title, _msg, LABEL_NULL, LABEL_NULL); \
showDialog(_type, NULL, NULL, NULL); \
showDialog(_type, NULL, NULL, NULL); \
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy & paste from PR #2603

kisslorand
kisslorand reviewed Jan 22, 2023
View reviewed changes
TFT/src/User/Menu/Popup.h
@@ -50,9 +50,8 @@ void _setDialogCancelTextLabel(int16_t index);
setDialogCancelText(canceltext); \
}

void popupDrawPage(DIALOG_TYPE type, BUTTON * btn, const uint8_t * title, const uint8_t * context, const uint8_t * yes,
const uint8_t * no);
//void popupReminder(DIALOG_TYPE type, uint8_t* title, uint8_t* msg);
Copy link
Contributor

Choose a reason for hiding this comment

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

"Borrowed" from Pr #2607

kisslorand
kisslorand reviewed Jan 22, 2023
View reviewed changes
TFT/src/User/my_misc.h
@@ -44,7 +44,7 @@ extern "C" {
#define SET_BIT_VALUE(num, index, value) num = ((value) == 1) ? (1 << index) | num : num & (~(1 << index))

// Toggle bit status at selected index
#define TOGGLE_BIT(num, index) num = num ^ (1 << index)
#define TOGGLE_BIT(num, index) num = num ^ (1 << index)
Copy link
Contributor

Choose a reason for hiding this comment

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

"Borrowed" from PR #2601

@kisslorand kisslorand mentioned this pull request Jan 22, 2023
Closed
@kisslorand
Copy link
Contributor

@bigtreetech
Please check out this: #2685. Thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kisslorand kisslorand kisslorand left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@digant73 @kisslorand