Display scroll an empty char if the requested char is out-of-range.

[microbit-carlos]

Currently it was scrolling a NULL pointer:

• For characters not in the font, display scrolls a null pointer codal-microbit-v2#423

This matches the V1 and MakeCode sim behaviour.

[JohnVidler]

Looks good, although do we want a blank char on an unknown glyph, or something more explicit, like a filled block, question mark, or other symbol?

Edit: or have a "badchar" constant we can use for any string conversions or stuff like serial IO

[microbit-carlos]

Looks good, although do we want a blank char on an unknown glyph, or something more explicit, like a filled block, question mark, or other symbol?

Edit: or have a "badchar" constant we can use for any string conversions or stuff like serial IO

Yeah, I had the same thought and proposed the question in:

• Should scrolling/printing out-of-range characters on the display be an "error glyph" codal-microbit-v2#425

Empty space right now matches V1, anything we'd need to consider as a "breaking" change (nor quite, but you know what I mean) and impact to things like the MakeCode simulator. We can continue that conversation over lancaster-university/codal-microbit-v2#425.

[microbit-carlos]

@martinwork also suggested we could apply this patch inside font.get(), so if we want to have configurable "error characters" it can be implemented in a single place.

[microbit-carlos]

@martinwork also suggested we could apply this patch inside font.get(), so if we want to have configurable "error characters" it can be implemented in a single place.

I was thinking about this, but one downside is that the BitmapFont::get() documentation explicitly indicates it returns "A pointer to the corresponding data buffer, or NULL if the character is not available". So it does make the caller responsible about what to do when a character is not supported, and changing this could probably be considered more of a breaking change for any other CODAL consumers.

[martinwork]

Ah, well spotted @microbit-carlos . That's out then!

This looks like a similar problem
https://github.com/lancaster-university/codal-core/blob/master/source/types/Image.cpp#L590

Could we add, say BitmapFont::getNonNull()?

[JohnVidler]

Having a 'safe' proxy for this situation makes sense to me.

I suggest a BitmapFont::getOrDefault( int codepoint, char default = CODAL_DEFAULT_BADCHAR ) schema for this, then we can have a CODAL default constant (space) while also allowing for application code to define their own if required with a local override, or for non-microbit-v2 targets to redefine the global symbol for platform-y reasons.

Personally I prefer the ...OrDefault suffix on methods where possible, as its simpler to understand than safeX or nonNull... and such.

[JohnVidler]

... having just written that, it now occurs to me that we could just extend the BitmapFont::get() method with a default argument for the bad glyph too, and we don't then increase the code footprint with a whole new method, just a minor signature change.

[microbit-carlos]

Thanks John, I think both options would work, but:

I was looking into implementing this as a default parameter, using the null char as the default, so that it can return a NULL pointer by default (as discussed in our standup). This way we could pass a character without casting or implicit casting.

const uint8_t* BitmapFont::get(char c, char defaultChar = '\0');

But this doesn't help if we wanted to change it in the future to be able to return an "error glyph" (lancaster-university/codal-microbit-v2#425). To be able to support that we'd need to end up using an int as the type (so -1 for NULL, -2 for the error glyph, etc and we end up with more and more if statements inside BitmapFont::get()):

const uint8_t* BitmapFont::get(char c, int defaultChar = -1);

And to be fair, this does not save us anything, as we still need to update all relevant calls to add/change the extra argument (from font.get(c) to font.get(c, ' ') now, and again when implementing the error glyph (otherwise it'd be a breaking change for other targets).
So I think we might as well merge this PR, which doesn't touch any CODAL API, and if and when we decide to implement the error glyph, then we can decide to do expand the API or not.

[microbit-carlos]

Reviewed in a call, good to go 🚀