Skip to content
New issue

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

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

Already on GitHub? # to your account

Bugfixes, restoration, cleanup, speedup, some FW size reduction #2776

Closed

Conversation

kisslorand
Copy link
Contributor

@kisslorand kisslorand commented Apr 19, 2023

Bugfix

  • In mesh editor menu if save button is pressed but than mesh saving to EEprom is cancelled and no more mesh edits are done, on the exit from the menu the user is not asked to save the EEprom. In the same menu if all the mesh values are under or above 0 than the minimum value or maximum value would be set to 0 and dot colors and size would be calculated wrong. These have been addressed, the edited values and the original ones are synced (copy current data over original data) only when mesh data is actually saved to the EEprom of the motherboard. Also the minimum and maximum values are correctly calculated even if all the values are all under 0 or all under above it.

  • In the "LevelingControl" API there was two typos which didn't affect the functionality, however they were corrected.

  • PR Fixed MeshEditor menu #2781 broke the circular column navigation in "MeshEditor" menu when using the screen touch controls (left, right); it has been restored without affecting the rotary encoder's linear navigation style.

Speedup

  • It bothered me that when exiting from mesh editor menu the screen has a pretty nasty lag, it gives the impression it is frozen. The lag of the popup about saving edited data when exiting is practically zero now. Also when saving mesh data to EEprom only the edited mesh points are synced (current data to original data).

  • The hide extension of the filenames in the file list procedure got a speed boost, by a rough estimation of at least 25% (the value is estimated, do not ask for scientific proof). The list creation and the navigation between pages in the list is more snappier. It is done by eliminating unnecessary extension hiding, restoration and nested functions.

  • The following expression type &string[strlen(string)] has been replaced by strchr(string, '\0'). The expression &string[strlen(string)] first finds the '\0' terminal character in the "string" variable counting in the meantime the bytes checked than iterates variable "string" by the bytes resulted in the previous check than it gives back the address of the iteration result, actually giving the address of the null terminator character of the "string" variable. strchr(string, '\0') does EXACTLY that with only one search, it directly gives the address of the null terminator character of the "string" variable.

Restoration, cleanup

  • The "strxcpy", "strwcpy" and "strscpy" alternatives for "strncpy" were restored. PR Bug fixes on top of last merged PRs #2768 eliminated them. PR Bug fixes on top of last merged PRs #2768 claims that "strxcpy is not applicable at all in VFS API". Well... "strxcpy" is EXACTLY there where it is best suited. It just adds that extra '\0' character needed for file extension hide and restoration functions.
    The author of that PR probably missed that the source string also contains the '\0' terminal character and the "strxcpy" copies only "num - 1" characters. So if the source is a filename (not a folder name) "num" equals to the number of bytes in the filename (excepting the '\0' terminal character) plus two more bytes (num = strlen(filename) + 2). If you substract 1 for "num" (becasue "num - 1" bytes will copy "strxcpy" from source to destination) you will end up to copy "num - 1" bytes from source which equals to "strlen(filename) + 1" which equals to the bytes number in the filename plus the '\0' terminator character. No risk of any overflow here.
    So not only that "strxcpy" is PERFECTLY SAFE and suitable for the job in VFS API but it is also faster compared to the offered alternative by PR Bug fixes on top of last merged PRs #2768. This also adds to the snappiness of the filelist.
    The alternatives offered by PR Bug fixes on top of last merged PRs #2768 are not as light as the ones they replaced and in some cases they fail to deliver the promised <"dest" always ends with '\0'> which results in an undefined/unsafe state of "dest" if it is not initialized beforehead. It's not the case with the functions of this PR.

  • "setPrintTitle()" is back reworked and with a very small footprint (40 bytes smaller than the original "getPrintTitle()").

  • PR Bug fixes on top of last merged PRs #2768 claimed to fix "wrong check on BedLeveling menu" but there was nothing to fix. Maybe it was just a lack of code understanding... If X can be only A or B and it is not B than what is X? :)
    Before checking if the firmware of the motherboard if it is Marlin or not there's a conditional if (levelStateNew != UNDEFINED) which limits the firmware to be either Marlin or Reprap. So if it's not Reprap, it's... you guessed right, it's Marlin! The fix of PR Bug fixes on top of last merged PRs #2768 was not a fix so it's reverted.

  • Notification API and the Settings got themself a touch of optimization resulting a smaller footprint.

  • other minor changes/adjustments

FW size reduction

It's 200+ bytes as a result of the above changes, and the size reduction of commit 2f2ed6b.

Notes

The changes described here were tested, they are not theoretical changes. However I am only human...

@kisslorand kisslorand force-pushed the MeshEditor-speed-mods branch 2 times, most recently from f371a5b to 9c88e0e Compare April 22, 2023 11:15
@digant73 digant73 mentioned this pull request Apr 22, 2023
@kisslorand kisslorand force-pushed the MeshEditor-speed-mods branch 3 times, most recently from 09c1675 to 6df3bfc Compare April 23, 2023 14:04
@kisslorand
Copy link
Contributor Author

kisslorand commented Apr 23, 2023

@everyone

It came to my attention that the self-proclaimed God of this repo has gone empty and has to steal codes and ideas again and again from ongoing PRs and just spice it up with empty space addition and deletion, capital/small letter changes and some code moving. To add weight and justification fixing of some fake bugs are added to the list of spices used. In my culture such a person is not called "a pillar of society", I do not know if this is the normal or not in his culture but anyway, I am confident he's just lost a little.

I really hope he will recover from this burnout and return to the coder he was. I'll include him in my prayers.... he might eventually recover,

@kisslorand kisslorand force-pushed the MeshEditor-speed-mods branch 14 times, most recently from 5301b95 to 71d9c3f Compare May 17, 2023 09:24
@kisslorand kisslorand force-pushed the MeshEditor-speed-mods branch 7 times, most recently from 14a228c to fb7f8d5 Compare May 25, 2023 20:32
@kisslorand kisslorand force-pushed the MeshEditor-speed-mods branch 4 times, most recently from 646e1de to 754e54f Compare May 30, 2023 10:48
@kisslorand kisslorand force-pushed the MeshEditor-speed-mods branch from 754e54f to e7953d3 Compare June 5, 2023 06:17
@kisslorand kisslorand force-pushed the MeshEditor-speed-mods branch 8 times, most recently from fca7a37 to 69a22a6 Compare June 13, 2023 17:05
@kisslorand kisslorand force-pushed the MeshEditor-speed-mods branch from 69a22a6 to d7a90e4 Compare June 13, 2023 17:05
@kisslorand
Copy link
Contributor Author

Closed due to breaking it up to separate PRs.

@kisslorand kisslorand closed this Jul 22, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant