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

Error in char map #22

Closed
holgerlembke opened this issue Nov 2, 2013 · 9 comments
Closed

Error in char map #22

holgerlembke opened this issue Nov 2, 2013 · 9 comments

Comments

@holgerlembke
Copy link

Today I wanted to write a °C ("degree Celsius"). So I looked up code page 437 and found it should be character number 248. But it did not work.

After some investigation I learned that the characters starting from 177 are shifted due to a missing character in the 176 to 178 range.

For easy reference have a look at the code page in Wikipedia http://en.wikipedia.org/wiki/Code_page_437 and compare it to this photo: http://forum.arduino.cc/index.php/topic,173537.0.html

0xb0/0xb1/0xb2 should have "gray" blocks but 0xb2 is missing.

Solution: in glcdfont.c add a line

    0x08, 0x14, 0x2A, 0x14, 0x22,
    0x22, 0x14, 0x2A, 0x14, 0x08,
190 0x95, 0x00, 0x22, 0x00, 0x95,  // <-----------
    0xAA, 0x00, 0x55, 0x00, 0xAA,
    0xAA, 0x55, 0xAA, 0x55, 0xAA,

and remove the five bytes at the end.

@PaintYourDragon
Copy link
Contributor

Ah ha. The missing character also likely also explains why the font[] array has only 255 lines (rather than 256). Thanks for spotting this.

This puts us in a sticky spot...add the missing glyph and break a fair amount of Adafruit_GFX code that's already out there (but with the library then Code page 437 correct), or live with it as a legacy bug (knowing all 'classic' code will continue to render symbols as it has). Perhaps best to add something like this around the missing glyph:

#ifdef CODEPAGE437
0x95, 0x00, 0x22, 0x00, 0x95,
#endif // CODEPAGE437
(The 5 bytes would remain at the end, they're actually correct)

Then NOT #define CODEPAGE437 by default; leave the old 'bad' behavior as the default, with the ability to switch it on for those needing it. Yeah, in a perfect world I'd rather be standards-compliant as the default case, but the foot is already shot at this point. Agree/disagree?

@holgerlembke
Copy link
Author

What code is broken?

If someone relies on the shifted characters I would force them to fix it.

Compromise: make it the other way round: by default the fixed solution and a #define for the old behavior. It will work this way with LCD-displays with hard coded charset, too. Otherwise we will have an eternal error.

In the end it is not satisfying at all. Totally doomed. I mean: I write a ° (degree) in the Arduino-IDE and get something strange on the display. I have to look up the code page 437 stuff (because I'm an old geezer I know this from ancient DOS to Windows text conversion times. Here we should think about the target audience of Arduino and all that stuff: do they really need to know all those details? Wouldn't it be better to spare them from this chaos?) and find it does not work. Then I go to google, ask question in a forum, perhaps find something, look up the code and activate an conditional define...

Real solution would be to have a proper ANSI code page that works from IDE to device... Wait. There is this MAC stuff, too. Argh.

So, stick with the code page 437 and make it right.

@holgerlembke
Copy link
Author

Additional note, more on the private discussion side due to the impossibility to write mails in github to you:

I try to write a small status display with about 12 lines of data. Currently I do

fillScreen(ST7735_BLACK);
println();
println();
println();
...

This does not look very good. If I omit the fillScreen() it overwrites the data lines where values change.

I thought that the println() perhaps blanks the area before writing, but as far as I see, it does not.

Any suggestions for a concept?

@PaintYourDragon
Copy link
Contributor

Dozens, hundreds, possibly thousands of customer projects at this point. The library's been adopted by Arduino for their "official" LCD now at Radio Shack and such, making this an "entrenched" bug. Doing the Right Thing in Principle (forcing the change) would frustrate an immense number of (otherwise satisfied) customers who might not understand the change (though yes, new users would benefit going forward). Not the first time something like this has happened. I'd suggest a transition in phases:

  1. 'Duct tape' fix as shown above (with the #define CODEPAGE437) along with a paragraph or so of comments explaining the mistake and the fix. All existing customers' projects continue to run as they did before. If they read up on those comments, they understand the issue, can enable the fix and update their code to match.
  2. Update the Adafruit_GFX library documentation, emphasizing the value of #defining CODEPAGE437 (makes it easy to look up a char map for the special chars), and update any examples to use the "good form." Over time, more and more customer code will then follow this methodology.
  3. Let it simmer; perhaps a few months to a year.
  4. Flip the switch so CODEPAGE437 is the default case, change the documentation so the "old way" (and how to fix it) is merely a footnote.

@PaintYourDragon
Copy link
Contributor

There's a version of setTextColor() that accepts two parameters, the second being a background color. If you set this to black, and pad out your strings to overwrite anything that might've been there before (e.g. sprintf() to a buffer first, then print that), this should achieve the desired effect without having to clear the whole screen.

@holgerlembke
Copy link
Author

I just found the second parameter of setTextColor()... Thanks!

@holgerlembke
Copy link
Author

Lets imagine what happens today if someone with the bugged version tries to use a character beyond the error. It won't work. So he has to look up the code page stuff, understand the problem (shifted by one but not all) and carry on. Apparently he/she did not write an error report. (Are there any?)

Is it really realistic that this had happened that much? I assume that it never happened before. Due to the In-IDE-it-is-different-than-on-display-problem I would assume all users did not see that problem because not using 8-bit-ascii.

So a fix would fix something that never happened.

But anyway, I pass the buck to you. :-)

Scott216 added a commit to Scott216/Arduino_Libraries that referenced this issue Mar 26, 2015
joshgoebel added a commit to joshgoebel/ArduboyLib that referenced this issue Jun 6, 2015
@PaintYourDragon
Copy link
Contributor

Resolved in latest commit. 'Wrong' behavior is still the default, so that all old code will continue to work for those users 100% as they expect. New code can simply call tft.cp437() to switch over to correct Code Page 437 indices for extended ASCII symbols (once, in setup(), is sufficient).

I apologize for how long this has festered. The extreme simplicity of a runtime workaround that doesn't break all existing code just didn't dawn on me. At some point in the future perhaps we can switch this over as the default behavior.

@hikari-no-yume
Copy link

Is it really realistic that this had happened that much? I assume that it never happened before. Due to the In-IDE-it-is-different-than-on-display-problem I would assume all users did not see that problem because not using 8-bit-ascii

I know this is an old bug but I want to point out that this happened to me. I noticed that the font was CP437, added a simple UTF-8 to CP437 translation table and then discovered that the box-drawing characters and beyond were incorrectly mapped :(

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

No branches or pull requests

3 participants