Closed edgar-bonet closed 3 years ago
Memory usage change @ ac0291bd448ae94b6850009e6f3abd484985c5f2
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
arduino:avr:leonardo | :small_red_triangle: +100 - +100 | +0.35 - +0.35 | :small_red_triangle: +2 - +2 | +0.08 - +0.08 |
arduino:sam:arduino_due_x_dbg | :small_red_triangle: +72 - +72 | +0.01 - +0.01 | N/A | N/A |
arduino:samd:mkrzero | :small_red_triangle: +76 - +76 | +0.03 - +0.03 | :small_red_triangle: +4 - +4 | +0.01 - +0.01 |
Please note that the checks that failed did not fail because of the changes introduced by this pull request (they also fail on master). They fail because the CI pipeline attempts to compile the examples for platforms that do not provide the HID.h header file.
LOVE THIS PR :heart_eyes:
Really, thank you for picking this up! I think the begin()
overload with the default on press()
is very sensible and is worth the extra flash occupation.
@agdl , would you mind testing it?
Has the review of this PR started?
I just realized that setting
_asciimap = KeyboardLayout_en_US;
in the constructor obviates the need to test within press()
that the pointer is not null. This would save three lines of code and 12 bytes of flash (on AVR). Should I add this change now although the pull request is already open?
Please, add it to this PR :slightly_smiling_face:
OK, done.
Memory usage change @ c609d4a7c6b9970f002c2903e75b3d708325adb5
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
arduino:avr:leonardo | :small_red_triangle: +88 - +88 | +0.31 - +0.31 | :small_red_triangle: +2 - +2 | +0.08 - +0.08 |
arduino:sam:arduino_due_x_dbg | :small_red_triangle: +64 - +64 | +0.01 - +0.01 | N/A | N/A |
arduino:samd:mkrzero | :small_red_triangle: +64 - +64 | +0.02 - +0.02 | :small_red_triangle: +4 - +4 | +0.01 - +0.01 |
This pull request adds support for multiple keyboard layouts. Five layouts are initially included: US, French, Italian, German and Spanish. It is expected that users contribute additional layouts (I can add a few if requested). Adding a layout is straightforward by following the instructions in KeyboardLayout.h. It only requires one file defining it, and one line added to Keyboard.h.
Context
Arduino was born in Italy and has become very popular all over the world. The USA is only one among many countries that show interest in Arduino, and yet, the official Arduino Keyboard library supports exclusively the US keyboard layout. This has caused much confusion (#7, #20, #23, #24) and user requests (#8, #21, #25, #50, #52).
In the past, requests to support international layouts have been rejected with two arguments:
Regarding point 2, it should be noted that:
This pull request addresses this need while keeping the code simple and small.
API change
The only change to the public API is that the
begin()
method now accepts an optional parameter specifying the requested keyboard layout, e.g.As an optional parameter, it defaults to
KeyboardLayout_en_US
, thus preserving compatibility with existing sketches.Even though calling
begin()
is mandatory according to the official documentation, this method has been a noop until now. In order to avoid breaking existing sketches that fail tobegin()
, thepress()
method now callsbegin()
if it hasn't been called before.Resource usage
For a sketch using the default layout, the cost of this change is 100 bytes of flash and 2 bytes of RAM, as measured on an Arduino Micro. This doesn't change as the number of supported layouts increases.
Using a layout other than the default costs an extra 128 bytes of flash. This is because the default layout is always loaded in order to handle the case where the user forgets to
begin()
before using the library. It could be discussed whether supporting sketches that fail tobegin()
is worth the cost.Limitations
Keeping this library simple and small is a must. Because of this, this feature comes with some limitations:
^
on some layouts) are not supported for the same reason. They also break the simple assumption thatwrite(character)
equalspress(character)
followed byrelease(character)
.