arduino-libraries / Keyboard

GNU Lesser General Public License v3.0
229 stars 160 forks source link

Keyboard Layout for Swedish #58

Closed PJ789 closed 2 years ago

PJ789 commented 2 years ago

.CPP file adds an ordinary Swedish keyboard layout. .h file adds scancodes for the non-ascii characters commonly used on Swedish keyboards (a-ring etc).

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

github-actions[bot] commented 2 years ago

Memory usage change @ 20f7498557c63454e4100e006d6c6cff6bdd6531

Board flash % RAM for global variables %
arduino:avr:leonardo 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:sam:arduino_due_x_dbg 0 - 0 0.0 - 0.0 N/A N/A
arduino:samd:mkrzero 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table Board|examples/Serial
flash|%|examples/Serial
RAM for global variables|% -|-|-|-|- arduino:avr:leonardo|0|0.0|0|0.0 arduino:sam:arduino_due_x_dbg|0|0.0|N/A|N/A arduino:samd:mkrzero|0|0.0|0|0.0
Click for full report CSV ``` Board,examples/Serial
flash,%,examples/Serial
RAM for global variables,% arduino:avr:leonardo,0,0.0,0,0.0 arduino:sam:arduino_due_x_dbg,0,0.0,N/A,N/A arduino:samd:mkrzero,0,0.0,0,0.0 ```
edgar-bonet commented 2 years ago

Oh, I almost forgot. You should declare the layout in Keyboard.h:

--- a/src/Keyboard.h
+++ b/src/Keyboard.h
@@ -116,6 +116,7 @@ extern const uint8_t KeyboardLayout_en_US[];
 extern const uint8_t KeyboardLayout_es_ES[];
 extern const uint8_t KeyboardLayout_fr_FR[];
 extern const uint8_t KeyboardLayout_it_IT[];
+extern const uint8_t KeyboardLayout_sv_SE[];

 // Low level key report: up to 6 keys and shift, ctrl etc at once
 typedef struct
PJ789 commented 2 years ago

Oh, I almost forgot. You should declare the layout in Keyboard.h:

You are indeed right, and I had indeed done that for myself, but overlooked it. Thanks Edgar.

PJ789 commented 2 years ago

Nice to see more layouts coming! 😄 I do not use a Swedish layout myself, but just tried to review this based on pictures I found on the Internet, and on trying to set that layout on my computer. Please, tell me if I made any mistake.

Edgar thanks very much for your detailed review. I'll double check the few items indicated above. The Swedish keyboard has some ambiguous (to a non-Swede) symbols and some unusual features versus a more familiar keyboard. The good news is that it is similar enough to Danish that, if we can get this going, I'm pretty confident I can do a Dansk version as well.

github-actions[bot] commented 2 years ago

Memory usage change @ cd9a30ca6c21154ce5025ace796a2a9ae6879c54

Board flash % RAM for global variables %
arduino:avr:leonardo 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:sam:arduino_due_x_dbg 0 - 0 0.0 - 0.0 N/A N/A
arduino:samd:mkrzero 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table Board|examples/Serial
flash|%|examples/Serial
RAM for global variables|% -|-|-|-|- arduino:avr:leonardo|0|0.0|0|0.0 arduino:sam:arduino_due_x_dbg|0|0.0|N/A|N/A arduino:samd:mkrzero|0|0.0|0|0.0
Click for full report CSV ``` Board,examples/Serial
flash,%,examples/Serial
RAM for global variables,% arduino:avr:leonardo,0,0.0,0,0.0 arduino:sam:arduino_due_x_dbg,0,0.0,N/A,N/A arduino:samd:mkrzero,0,0.0,0,0.0 ```
edgar-bonet commented 2 years ago

The Swedish keyboard has some ambiguous (to a non-Swede) symbols and some unusual features versus a more familiar keyboard.

You mean the dead keys at the top right which are used for diacritics? We have dead keys on the French, German and Spanish layouts as well.

PJ789 commented 2 years ago

Are we done? How do we get this into the master branch?

PJ789 commented 2 years ago

"Only those with write access to this repository can merge pull requests" ... anyone?

edgar-bonet commented 2 years ago

I tested this pull request (commit cd9a30ca6c21154ce5025ace796a2a9ae6879c54) with the following code:

#include <Keyboard.h>

extern const uint8_t KeyboardLayout_sv_SE[];

void setup() {
    Keyboard.begin(KeyboardLayout_sv_SE);
    delay(5000);
    Keyboard.print("<>\\|");
}

void loop(){}

I switched, on my PC, my keyboard layout to Swedish, then uploaded the sketch to my Arduino Micro, and a few seconds later the Arduino typed the following:

<{/€

This shows that:

Could you please run this sketch on your setup? If you get the same results, that would mean that at least the codes for >, \ and | should be fixed. For consistency, it wouldn't hurt to also fix <.

I ran the same test with the fixes I proposed and I got the correct result, namely <>\|.

PJ789 commented 2 years ago

Using SWE layout I get same (<{/€). OK. I'll fix that (I have a SWE test keyboard handy).

edgar-bonet commented 2 years ago

Also, don't forget to declare the layout in Keyboard.h.

PJ789 commented 2 years ago

OK... one difference... it works in my project code when I use Keyboard.press() But not when I use keyboard.print(). Still investigating... but I think there may be some difference between the two methods...?

edgar-bonet commented 2 years ago

it works in my project code when I use Keyboard.press() but not when I use keyboard.print().

That should not make a difference. I modified my test sketch like this:

void setup() {
    Keyboard.begin(KeyboardLayout_sv_SE);
    delay(5000);
    Keyboard.press('<');  Keyboard.release('<');
    Keyboard.press('>');  Keyboard.release('>');
    Keyboard.press('\\'); Keyboard.release('\\');
    Keyboard.press('|');  Keyboard.release('|');
}

and got the same result: <{/€.

PJ789 commented 2 years ago

My suspicion at the moment is that Keyboard.press('<') plus a separate keyboard SHIFT modifier keypress works... which is how my current project functions (and that is fine).

Where as Keyboard.press('>') lookup in KeyboardLayout_sv_SE returning 0x64|SHIFT as a single code, attempting to send that combo (which might relate to the comment in KeyboardLayout.h ("The exception is 0x64, the key next next to Left Shift on the ISO layout (and absent from the ANSI layout). We handle it by replacing its value by 0x32 in the layout arrays.")

Which you did highlight, and I didn't understand... but now I still don't understand... but can see is relevant. I will try using 0x32 and see if that solves it :)

PJ789 commented 2 years ago

OK, its fixed, this code was used to test;

`#include

extern const uint8_t KeyboardLayout_sv_SE[];

void setup() { Keyboard.begin(KeyboardLayout_sv_SE); delay(5000); Keyboard.press('<'); Keyboard.release('<'); Keyboard.press('>'); Keyboard.release('>'); Keyboard.press('\'); Keyboard.release('\'); Keyboard.press('|'); Keyboard.release('|'); Keyboard.print("<>\|"); }

void loop(){}`

And yields the right results now. I'll fix the sv_SE layout.

PJ789 commented 2 years ago

Hi Edgar...

thank you. I believe all queries are resolved, your contributions were quite correct.

Thank you very much for your help & review.

Are you able to commit this contribution to the master branch now?

regards Pete.

github-actions[bot] commented 2 years ago

Memory usage change @ 28735fda3beecb468b2ce08dd37b6197d832b9bb

Board flash % RAM for global variables %
arduino:avr:leonardo 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:sam:arduino_due_x_dbg 0 - 0 0.0 - 0.0 N/A N/A
arduino:samd:mkrzero 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table Board|examples/Serial
flash|%|examples/Serial
RAM for global variables|% -|-|-|-|- arduino:avr:leonardo|0|0.0|0|0.0 arduino:sam:arduino_due_x_dbg|0|0.0|N/A|N/A arduino:samd:mkrzero|0|0.0|0|0.0
Click for full report CSV ``` Board,examples/Serial
flash,%,examples/Serial
RAM for global variables,% arduino:avr:leonardo,0,0.0,0,0.0 arduino:sam:arduino_due_x_dbg,0,0.0,N/A,N/A arduino:samd:mkrzero,0,0.0,0,0.0 ```
edgar-bonet commented 2 years ago

Hi Pete!

I do not have write access to this repository. I am just the contributor who implemented the support for multiple keyboard layouts, and the layouts fr_FR, it_IT, de_DE and es_ES (c.f. PR #53). This is why I am very glad to help others expand this support.

BTW, do not forget to add your layout to Keyboard.h.

PJ789 commented 2 years ago

Thanks again Edgar, a revised keyboard.h is also attached now.

I hope someone will merge this pull request for us?

github-actions[bot] commented 2 years ago

Memory usage change @ c62a5c7f6cc72f75622d2e0ee5969a9745c07707

Board flash % RAM for global variables %
arduino:avr:leonardo 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:sam:arduino_due_x_dbg 0 - 0 0.0 - 0.0 N/A N/A
arduino:samd:mkrzero 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table Board|examples/Serial
flash|%|examples/Serial
RAM for global variables|% -|-|-|-|- arduino:avr:leonardo|0|0.0|0|0.0 arduino:sam:arduino_due_x_dbg|0|0.0|N/A|N/A arduino:samd:mkrzero|0|0.0|0|0.0
Click for full report CSV ``` Board,examples/Serial
flash,%,examples/Serial
RAM for global variables,% arduino:avr:leonardo,0,0.0,0,0.0 arduino:sam:arduino_due_x_dbg,0,0.0,N/A,N/A arduino:samd:mkrzero,0,0.0,0,0.0 ```
PJ789 commented 2 years ago

Hi @agdl .. I hope I've done the right thing. I've 'assigned' this PR to you, in the hope you could merge his enhancement that Edgar & myself have created... adding support for Swedish keyboards/scancodes to Keyboard. regards Pete

PJ789 commented 2 years ago

@facchinm are you able to merge this enhancement that Edgar & myself have created adding support for Swedish keyboards/scancodes to Keyboard? regards Pete

facchinm commented 2 years ago

Since it's been approved by @edgar-bonet I'm squashing and merging it :slightly_smiling_face: Thanks for your contribution!