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

Limit status message display taking multi-byte characters into account #6855

Closed
wants to merge 1 commit into from

Conversation

tcm0116
Copy link
Contributor

@tcm0116 tcm0116 commented May 25, 2017

This PR reverts the changes made in #6816 and implements my proposed solution in the comments for that PR.

To test this on my machine with a 20x4 LCD, I set the machine name to "MACHINE NAME ó" and set the language to "ca" (the 'ó' character is a two-byte Unicode charter). It would be a good idea of someone with a DOGM display could perform similar testing.

This first image shows the result with #6816 applied, and it can be seen that the status message is missing one character.

img_20170525_085438

This image shows the change in #6816 reverted.

img_20170525_085723

This image is with this PR applied.

img_20170525_090843

For this image, I removed the special character from the machine name to verify the message is clipped properly with​ only single-byte charters in the string.

img_20170525_091214

And finally, I set the machine name to a short value to show that messages less than LCD_WIDTH are displayed properly.

img_20170525_091405

@tcm0116
Copy link
Contributor Author

tcm0116 commented May 27, 2017

@Roxy-3D Since I know you have a DOGLCD, would you mind testing this if you have a chance?

@Roxy-3D
Copy link
Member

Roxy-3D commented May 27, 2017

I'll try to remember to do that... I have a very long print running right now, so it will be tomorrow before I can do that.

@thinkyhead
Copy link
Member

thinkyhead commented May 27, 2017

The reason I limited the change only to the printing of the status message was to avoid adding any overhead to the lcd_print functions themselves, which are used to print just about everything on the screen. We recently did a heavy optimization to the LCD code, and I would like to continue to strip cycles. I suggest that if we want a function that takes a maxlength, we should add that as a new and separate function, and only use it in those instances where a string might potentially be too long (or if it needs charset_mapper).

@thinkyhead thinkyhead added Needs: Patch A patch is needed to fix this PR: Bug Fix and removed T: Suggestion labels May 27, 2017
@thinkyhead thinkyhead closed this May 28, 2017
@tcm0116 tcm0116 deleted the lcd_print branch September 24, 2017 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Patch A patch is needed to fix this PR: Bug Fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants