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

Rework i18n mechanism and add IT layout #231

Merged
merged 6 commits into from
Mar 5, 2021
Merged

Conversation

SukkoPera
Copy link
Contributor

@SukkoPera SukkoPera commented Aug 30, 2020

This is the new way of handling keyboard layouts I proposed in #230.

Only this one or that one should be merged, eventually.

If this is chosen, the DE keymap must be updated before the merge EDIT: Updated

@SukkoPera SukkoPera mentioned this pull request Aug 30, 2020
@NicoHood
Copy link
Owner

Its been long time, that i've been working on this, so please help me once more...

Are you missing some files here? This does not look like a huge change, just an addition of italien layout.

@SukkoPera
Copy link
Contributor Author

Please read #230 carefully, it's all there.

SukkoPera added a commit to SukkoPera/HID that referenced this pull request Feb 2, 2021
@SukkoPera
Copy link
Contributor Author

SukkoPera commented Feb 2, 2021

This PR is finally complete, please consider for merging. I think this way of handling international keyboards is easier to manage.

KEY_IT_EGRAVE|MOD_LEFT_SHIFT|MOD_RIGHT_ALT, // {
KEY_IT_BACKSLASH|MOD_LEFT_SHIFT, // |
KEY_IT_PLUS|MOD_LEFT_SHIFT|MOD_RIGHT_ALT, // }
KEY_IT_IGRAVE|MOD_RIGHT_ALT, // ~ (Linux only: This works in X11, in console it's KEY_0|MOD_RIGHT_ALT)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Italiens do not have a tilde? O.o

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately we don't, and we don't have a backtick either. At least Linux has some easy combinations for those (even though it's always annoyed me that console and X11 use different ones, grrrrr!), but on Windows you get into crappy ALT+numpad stuff, which is a nightmare on notebooks :/.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thatswhy I am using US layout for coding instead of german layout ;-)

@NicoHood
Copy link
Owner

NicoHood commented Feb 4, 2021

Hey @SukkoPera thanks for taking the time to rework your PR! I stared at it for quite some time (it's been a long time since I've been working on this library), but your proposal makes absolutely sense to me. The unified keycode table to press a specific key language independently.

Did you test, that all usual characters are still working fine? What about writing a simple test sketch that we can add to the examples? It could type all characters from space to tilde (poor italiens, no tilde :-P). Then it would be easier to add new layouts and also test them.

Could you please try to verify that all (modified) source files contain spaces, not tabs? I imagine they might be mixed (by me years ago), but they are displayed with mixed indention in Github. Doing a bit of beautifying would be nice, if you like to.

Other than that, I'd say: great work! Did you have any chance to invite someone else to try this out? Maybe someone who is also using this library? Would be nice to hear some additional feedback :-)

@NicoHood
Copy link
Owner

NicoHood commented Feb 5, 2021

Did you read me last comment? (Just in case you overlooked that)

@SukkoPera
Copy link
Contributor Author

I did and I agree, but I need some time before I can do that, sorry. Will be back to it next week!

@NicoHood
Copy link
Owner

NicoHood commented Feb 5, 2021

Alright, no problem :-)

@NicoHood
Copy link
Owner

Ping

@SukkoPera
Copy link
Contributor Author

Pong! Sorry but I've been halted by some dental problems. I will work on this in the weekend, I promise :).

@SukkoPera
Copy link
Contributor Author

Example added. It's not the prettiest but seems to be working very well. Feel free to modify it as you prefer.

delay (5);
BootKeyboard.release(KEY_ENTER);
delay (5);
while (!Serial.available())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a single row for the semicolon.

Serial.println ("will be the same on every line. If they are not, try again with a different keymap.");
Serial.println ("DO NOT CHANGE WINDOW UNTIL THE SKETCH IS DONE!!!");
Serial.println ("Please click in the input box above, make sure it is empty and press ENTER");
while (!Serial.available())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a single row for the semicolon.

Serial.read();
Serial.print (r);
if (r == c) {
Serial.print(" OK");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a nice idea!

@NicoHood
Copy link
Owner

NicoHood commented Mar 4, 2021

Very nice idea! I've added some comments about some cosmetics. I think the PR is ready than. Thanks a lot for looking into that again :-)

@SukkoPera
Copy link
Contributor Author

I've taken care of all your observations and rebased the branch on your master. I think it's ready for merging too :).

@NicoHood NicoHood merged commit 499bbba into NicoHood:master Mar 5, 2021
@NicoHood
Copy link
Owner

NicoHood commented Mar 5, 2021

Thanks a lot

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

Successfully merging this pull request may close these issues.

2 participants