Enahancement: As a user user I need to define a character for undefined characters, or change the segment mapping for an existing character

[fourstix]

Enhancement (Request for Review and Comment)

As a user user I need to define a character for undefined characters, or change the segment mapping for an existing character.

Please see this issue in my fork of this repository https://github.com/fourstix/SparkFun_Alphanumeric_Display_Arduino_Library/issues/1

Your workbench

• Sparkfun Redboard with Qwiic
• Qwiic Alphanumeric Display - Blue SPX-16426
• Sparkfun_Alphanumeric_Display_Library v1.0.0
• Qwiic Cable
• Redboard powerd via USB; Display powered via Qwiic

Steps to reproduce

Please see this issue in my fork of this repository for the enhancement code and more details. https://github.com/fourstix/SparkFun_Alphanumeric_Display_Arduino_Library/issues/1

I welcome your comments and suggestions before I create a pull request with this function.

This code has been tested with all the examples and is ready to be merged. However I would appreciate any suggestions to improve the code before merge.

Proposed solution:

1. Move character mapping table from function static variable to class static
2. Expand character mapping table array from 90 to 96, to cover all printable characters
3. Add a new function defineChar(char c, uint16_t segment_map) to change the segment mapping data in the array.
4. The defineChar function should accept characters from '!' (33) to '~' (126)
5. The segment_map should be masked to 14 bits representing segments a through n.
6. Space (32) should remain blank and DEL/Unknown (128) should remain all segments on, or '0x3FFF' in 14 bits.

Expected behavior

Adding this function to the library will allow users to update the character segment mapping to display their own graphics for a character.

Actual behavior

Today there is no function available to do this. Character graphics is a matter of taste, and different users will have different preferences. Users unsatisfied with a character on the display, would be able to change it easily.

[nseidle]

fourstix! Good to see you over on alphanumeric. This looks like a great feature add. A few things:

• Looking at your commit, you are mixing a few things together (removing colonOn/Off functions, etc). Please separate into discrete PRs if possible.
• The segment array should be (and should have been) inlined so that it gets loaded into progmem not RAM. This runs counter to what you're trying to do so perhaps we can create a scratch area (8 custom chars, 16 bytes RAM should be enough?) where users can define custom characters. I'd love to limit the amount of RAM used.

[nseidle]

I added a handful of comments on the commit as well.

If you need hardware (additional displays) please let me know. We welcome the help!

[fourstix]

Thank you, Nate. I made a mistake in using the code obtained from the Arduino Library Manager to make the changes. The Arduino Library Manager seems to give an older version of the code in GitHub. I should have compared them and not assumed it was up to date.

So my code update wipes out a previous commit. I'm going to re-base my code locally, and redo the changes to commit the right code. Thank you for catching this!!!

[nseidle]

If I had a nickel for every time I had to rebase! No worries.

[fourstix]

Haha, Thanks. I now have the code at the right level. If you do a compare you should see only the changes I made. I ran the examples as a regression, and everything looks good.

By the way, I see a compiler warning in the Arduino IDE: Invalid version 'v1.0.1' for library in: C:\Users\Gaston\Documents\Arduino\libraries\SparkFun_Alphanumeric_Display_Arduino_Library

I think is because the official Arduino Library Manager library is still at 1.0.0. Do you want me to open a separate issue for that so someone can get the Arduino team to update the Library manager?

[fourstix]

One question. I thought it might be good to add hex constant values for SEG_A, SEG_B ... SEG_N so that one could write display.defineChar('z', SEG_N|SEG_G|SEG_D); rather than display.defineChar('z', 0b10000001001000); The constants could be used with the illuminateChar() function as well.

Do you think that would be a good idea? Please let me know, it would be easy to add them.

[nseidle]

A very good idea and has no memory footprint. Please do!

[fourstix]

Done! Please let me know if you have any suggestions or changes, or if you think it's ready for a pull request. Thanks!

[nseidle]

Please see one of my earlier comments:

The segment array should be (and should have been) inlined so that it gets loaded into progmem not RAM. This runs counter to what you're trying to do so perhaps we can create a scratch area (8 custom chars, 16 bytes RAM should be enough?) where users can define custom characters. I'd love to limit the amount of RAM used.

Do you think a uint8_t customChar[2 * 8] is a good idea?

[fourstix]

Hmmmmm.... That would make printing a bit tricky. Wouldn't one have to search each every character against that lookup array for a possible replacement before writing them to the display? Your array would also have to be more of a hash or look up table, as you would need to store each character being replaced along with it's mapping.

I'm struggling to see exactly how that would work. Right now it's easy to just over-write the table entries, since the table isn't in Progmem. But if we inline the table, is there a way to redefine or redirect certain entries? After all, this is all a matter of taste. I want to replace a,f,e,s and z with mappings I prefer. But someone else would want to map other characters, and might prefer to leave those alone.

Is the table that big? I think it's 192 bytes total, counting 14 bits as two bytes. Is that big in Arduino-land?

[fourstix]

Ugh. Wait. Asking "Is the table really that big?" has all the desperation and denial of "Do I look fat?" The unpleasant answer is "Yes, everyone knows it." So I need to go back and refactor this solution design.

I did some more thinking about this and I think a better approach would be: 1) Inline the table to save room for the user's own code. (That's a requirement.) 2) Define a DefChar object to hold a character definition 3) Add a linked list to the class to hold any DefChar objects.

• Normally the list ptr would be null
• Generally the number of defChar in the list will be small (5 or less) 4) Create a function to get a character segment map
  • Search the list for a new char definition
  • if found return the new definition for the character
  • if not found in list return the default map from the table 5) defineChar would be changed to create a DefChar object and add to the list

I think this would perform satisfactorily. Given that the large majority of users would have no character definitions in the list, the usual overhead is a single null check. A few users may have one or two definitions in the list, and a few oddballs like me may go up to 4 or 5. Searching a list of 5 items is not bad overhead. I seriously doubt anyone would go up to 20, which is the rule of thumb limit for a brute force (British Museum) search.

I admit this is a bit more ambitious than I intended. I need sometime to refactor code, and refresh my memory on Linked Lists.

Do you know of any good examples of in-lining static data in an Arduino Library? I know how to use PROGMEM and pgm_read_byte_near() in Arduino programs, but I've never done that in an Arduino library itself. I can probably find one by searching. Maybe I just include the right header and use these.

What are your thoughts on this approach? It sounds like a fun coding project to me.

[fourstix]

OK, moving to PROGMEM turned out to be pretty easy. Once I included the header, it just worked. There's even a pgm_read_word_near function so I didn't have to do any math on the offset.

Would you prefer this in two pull requests? 1) Update and move the Table to Flash then 2) Add the defineChar function or just one pull request with everything. It's easy for me to package it whichever way you prefer.

[fourstix]

Fixed a couple of minor issues I found in regression testing.

1) Commented out a stray debug print statement at line 275 in decimalOn() function. 2) Changed the character segments in the table for period and colon to blank. The original mappings for colon and period had segment A illuminated which would over-write other characters in the display when colon or period was printed. (The code turns on the fixed period or colon, and doesn't advance the cursor. So next character overwrites the position the period or colon was in.) This fixes the odd appearance when a colon or period is in the print string.

[fourstix]

Code is done, tested and ready for review.

[oclyke]

Just reading along - excellent work @fourstix. That's a well thought out solution. Leaving any action up to Nate however (not even sure why GitHub notified me about this repo)

[makin-stuff]

Merged the pull-request.