Skip to content

Conversation

@kamilkrzyskow
Copy link
Contributor

@kamilkrzyskow kamilkrzyskow commented Mar 26, 2025

Hi 👋 ,

Before the rebase onto develop-HEAD, I did check on my develop branch that the CI passed:

The current develop-HEAD doesn't pass CI, so I haven't run it on the fork, but I built the app locally and side loaded the debug app onto my phone, and all relevant functionality is preserved.

I also wanted to add a Polish resource string, but the current Crowdin setup appears to be somewhat broken as it invalidated every translation.

  • <string name="dialog_message_keycode_to_scancode_trigger_explanation">Naciśnięty przycisk nie został rozpoznany przez system wejściowy. W przeszłości Key Mapper wykrywał takie przyciski jako jeden i ten sam. Obecnie aplikacja próbuje rozróżnić przycisk na podstawie kodu skanowania, który powinien być bardziej unikalny. Jest to jednak prowizoryczne, niekompletne rozwiązanie, które nie gwarantuje unikalności.</string>

Thanks to this I've noticed that it's likely a requirement to add translations via Crowdin, as it's a one-way pipeline for translations into the repo, and it doesn't go the other way, so I can't update translations via PR.

So just to be sure I need to create a Crowdin account to join in the translation involvement? Then again 90% was finished before, so I'm not sure if I should butt in with my unsolicited advice, even though I've seen some "controversial" translations, but also nothing directly incorrect 😅

I use English on my phone, so I didn't notice the Polish translations at first, but when I switched I noticed some missing strings.
Then after looking into the code it turned out they're hardcoded:

As for the Shizuku stuff mentioned before in the Issue, I've installed it, and thanks to this the accessibility service auto restarts after app reinstall 👍. Then I looked into intercepting the input events from Shizuku and came up short, especially since it would have to be injected in the correct place to go well with the current keyCode pipeline or would have to replace most of it, and then you mentioned adding custom getevent handlers and other low level linux things, so this topic is likely beyond my skill set / time investment capabilities at the moment. Therefore, I hope this PR will do ✌️

I can move the CI/build related stuff to another PR if you want, but I don't think it's necessary, so I kept it here.
Or maybe I should squash the 2 Issue related commits into one and keep only 1 commit + the CI/build commits?

Fixes #1276
I haven't checked the other #1277 Issue, as responding to keys while screen is off requires root access?

@sds100
Copy link
Collaborator

sds100 commented Mar 26, 2025

Thanks! I invalidated all the strings because in version 3.0 I changed so much stuff that the translations were down to 60% or lower and will need to wait before they are complete again. A lot just changed on develop so I hope the conflicts weren't an issue.

@kamilkrzyskow kamilkrzyskow force-pushed the fix/scancode-fallback branch from 9c19a80 to cb863bb Compare March 26, 2025 11:48
@kamilkrzyskow
Copy link
Contributor Author

kamilkrzyskow commented Mar 26, 2025

A lot just changed on develop so I hope the conflicts weren't an issue.

I got lucky, and had no major issues with the rebase. All of these force-pushes are from my own mistakes, like not running unit tests / code style locally / some nitpicks etc.

BTW I'll not have steady access to my "Android Studio capable" PC after Saturday for the next ~2 weeks, so I won't be able to ideate / work further on the PR after this, or maybe I will be able to, but with a time delay ✌️

@kamilkrzyskow kamilkrzyskow force-pushed the fix/scancode-fallback branch 2 times, most recently from 7a4cef2 to 2bfc15b Compare March 26, 2025 12:22
@kamilkrzyskow kamilkrzyskow changed the title Add scanCode fallback for unknown keyCode #1276 refactor: fallback to scanCode when keyCode unknown Mar 26, 2025
@sds100 sds100 self-requested a review March 26, 2025 18:41
Copy link
Collaborator

@sds100 sds100 left a comment

Choose a reason for hiding this comment

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

Thank you for github actions tweaks and the documentation fixes as well :)

@kamilkrzyskow kamilkrzyskow force-pushed the fix/scancode-fallback branch 2 times, most recently from 5a1d6ce to 4428ae6 Compare March 28, 2025 22:58
@kamilkrzyskow kamilkrzyskow force-pushed the fix/scancode-fallback branch from 4428ae6 to 30f5d8b Compare March 29, 2025 00:28
@kamilkrzyskow kamilkrzyskow requested a review from sds100 March 29, 2025 00:28
@kamilkrzyskow
Copy link
Contributor Author

Is there something else that I need to do? Is a unit test a requirement for this PR to be merged?

@sds100 sds100 added this to the 3.0 milestone Mar 29, 2025
@sds100 sds100 merged commit a18161c into keymapperorg:develop Mar 29, 2025
3 checks passed
@kamilkrzyskow
Copy link
Contributor Author

Thanks for the merge ❤️

@kamilkrzyskow
Copy link
Contributor Author

kamilkrzyskow commented Mar 29, 2025

So I've been setting up my key maps for the usage on the trip during the next 2 weeks.
And only now have I realized that using getMaxKeyCode is incorrect, because it would change the offset depending on the max code, and therefore a different final scan code would break the trigger detection.

Sorry for noticing it after the merge @sds100, should I make another PR or will you make a fast fixup commit?

If 1000 seems too small, then anything below 10000 would still suffice, considering that after years of Android APIs we have only 350 buttons. and AI devices won't have any buttons 😄

@sds100
Copy link
Collaborator

sds100 commented Mar 29, 2025

I'll do it quickly :)

@sds100
Copy link
Collaborator

sds100 commented Mar 29, 2025

Can you check the latest commit on develop

@kamilkrzyskow
Copy link
Contributor Author

Yup, fetched the changes and the triggers stopped working again, due to scan code change, so I had to update the triggers.

Thanks for fast commit revert ✌️ 🥳

As for why I haven't noticed it before, the offset got smaller from 1000 to 350, so I just brushed it off as invalid if >= offset conditional, instead of seeing it as not exact match 🤦

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.

Distinguish CAT S22 Flip function buttons

2 participants