device: support backlight on MX Keys S #2230
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pfps
force-pushed
the
backlight
branch
2 times, most recently
from
3daa9af to
17c9ac2
Compare
pfps
changed the title
|
Originally I monkey patched the first commit of this PR into my system solaar and the backlight toggle was working great. Since your new commits I've pulled this PR using git, but I'm now gettting |
|
Please run as |
|
Also, there was a bug in the code you used so please download the current version of this PR. |
|
Setting the backlight to "Disabled" initially worked, except setting it to "Software" the backlight level slider would not work. I then tried setting the various duration sliders and they all gave errors, and now I the backlight dropdown no longer works, gives the same error as earlier. Log: |
|
The option results in a Read/Write error and doesn't commit, although Solaar seems to save it, as when I restart solaar it will then commit the last option that previously failed. Log after the fail: |
|
OK, a couple of new bug fixes are in the PR. Please run again as Check changing the backlight level when the backlight is on software and when it is on auto. |
|
Disable mode was working and Auto was working (backlight slider did nothing) but then I got a FeatureCallError when switching to Software: I ran solaar again and it was in Software mode now and the backlight slider works (as well as the keyboard keys FN+F4/F5) but then switching to manual gave the FeatureCallError for any future changes: When I restarded again in Manual mode the backlight slider no longer worked bu the Fn+F4/F5 keys did, which I believe is intended. Looks mostly good, just the FeatureCallError becomes blocking when changing between some settings. |
|
The Backlight Delay Hands In/Out options appear to work fine as well, although the Hands In option ended up giving an error after fiddling with it for a bit and I couldn't change it any longer: |
|
OK, this is progress. Try again with the current version of the PR. Try switching to software mode with brightness level 7 and then brightness level 0. There was an off-by-one error in the code. Also, try switching between modes with brightness level 0 and then with a non-zero brightness level. |
|
It's not working very well. Still getting FeatureCallError when switching between the modes and then can't do anything until I restart solaar and it will commit whatever mode it failed previously. and now sometimes the backlight slider isn't working even in software mode, seems to just turn off once I turn it down. |
|
It's partly working. Try setting to Software, then change level to 0, then change to Auto, then change to Manual, then change level. Restart. Then try setting to Software, then change level to 2, then change to Auto, then change level to 0. It may be that level can be non-zero only in Software more. All this appears to stem from some nasty constraints in the firmware and some silly choices in the interface. |
|
Does changing the level in Software mode actually change the brightness of the backlight? |
Sometimes, but then it would stop working after adjusting the brightness slider and a FeatureCallError. EDIT: Does not appear to be working at all now even in Software mode anymore: The differentiation between "Manual" and "Software" is also a bit confusing, in Logi Option+ there was two modes (Auto/Manual), the slider would follow the keyboard state when using the FN keys to adjust brightness in manual mode, or it was in "Auto" mode and the slider would be disabled. I will see if I can get some screens of the software if you want them. Also, I get this error pretty much any time I change the mode, this was setting it to disabled: Perhaps change back the combobox to the last state before a FeatureCallError (or before committing it), or query the state again by querying the device (understandable if thats a bit trickier right now) - that way I don't have to keep restarting solaar. EDIT: I can't ever unlock the Backlight combo box until I restart Solaar, because I get this error: |
|
The problem in your latest run appears to be that one of the durations was 0, which is not valid. The current version of Solaar prohibits that value, so please download again and retry the test. You should edit |
|
The slider seems to work on "Solaar" mode now. Feels kind of janky though. If I slide it down to 0 the backlight goes off, but won't turn back on again when I turn the slider back up, UNTIL I put my hands on the keyboard for the prox sensor. Get an error when switching to "Keyboard" mode: |
That looks like a "feature" of the device, not anything that Solaar can affect. |
|
Try changing to Keyboard and Auto mode after setting the level to 0. It looks as if having level non-zero causes an error in these modes. |
|
The implementation of this feature is really annoying. I'm going to try to see if I can get information on what transitions are allowed. You would think that it would be possible to change to disabled from any setting. I'm going to put together some commands for hidconsole in an attempt to see what can and can't be done. |
|
On a very close inspection of the feature documentation, I see that software cannot set keyboard mode. I suppose that this mode is only for when the user sets the level using a key on the device. I'm going to update the Solaar code. |
|
Run a few tests through the UI:
|
Yeah I mentioned this previously in the Logi Options UI, iirc they just have the Automatic mode on or off, and having it on disabled the Fn keys. Maybe remove the I just tested and I don't get any errors if I don't switch to keyboard mode, so the only thing that would be nice would be to have the backlight slider state update with the FN Keys in Manual/Solaar mode. |
|
So in Software mode, a pair of Fn keys change the backlight level? Do they do as well in Automatic mode? It is often the case that there are changes that can be made to devices that are not possible using the Logitech software. The current version of the PR remove the manual mode and outputs more device information with |
|
The current version of the PR gets the changed backlight levels and tries to update the backlight slider. The interface that it uses hasn't had much testing so there may be errors. Run |
Yes they work in
This version worked well with no errors, although it took longer to startup, looked like you're querying the modes. Did some further testing and one more thing to note is when I change the backlight slider it often triggers the "hands out" so the backlight just turns off until I move my hands away and then back in again. |
|
Actually latest build gives an error when using the Fn keys to adjust brightness: It also slow my computer a bit with all the querying of the device. |
|
There shouldn't be many extra device commands, except for some testing code that will be removed. The error is caused by an incorrect usage of a rarely-used interface. I'm seeing the error in another feature so I'll debug it there (which should be quick). The error should only affect update of the slider, nothing else. |
|
The error should be fixed. |
|
Please try various changes with |
|
No error anymore, however when I press FN+F4 until its on the lowest, solaar switches to Also the backlight slider isn't changing with the FN keys. And solaar starts with the backlight mode empty now: Log: EDIT: I also notice in the log its evaluating two rules twice with only one of the Fn keys press:
Not sure if thats right. |
|
Rules are triggered on each notification. I don't see multiple evaluation. The logging for rules is very chatty and shows the entire rule, not just the top level of the rule. So the following bit of your log looks fine: |
pfps
force-pushed
the
backlight
branch
2 times, most recently
from
abcaec0 to
30a0121
Compare
|
The current version of the PR changes the setting from Solaar to Manual. It also has more debug logging of UI changes in response to backlight level changes using the keys. The backlight slider should change in response to changes using the keys. Run as The empty value on startup is likely due to an invalid value in your config file. If you change the setting then this should not recur. If it does, provide the config file. It would be nicer to have the level slider locked except in Manual mode, but this would require some work. The tooltip does mention that changes are only in effect in Manual mode. |
|
Slider still not following Fn Keys. Still switching by itself to Automatic mode when Fn+F4 to the lowest setting, then back to Manual mode when FN+F5. Log: |
|
Both these problems are caused by a silly bug - a level change notification causes a change to the backlight setting - not the backlight level setting. :-( Should be fixed now. |
|
Perfect, slider is moving with the FN Keys and the backlight mode isn't switching by itself anymore. Only issue is sometimes if I move the slider down a few values, it doesn't seem to commit the change, the slider will jump up several values after tapping the FN+F4(increase) key again. This mostly happens in automatic mode, the last few changes in the log here: |
|
I expect that the delays are the result of a combination of debug logging and the extra debugging reads. I removed the extra reads in the current version. Try with debugging off and see whether the problem persists. But there are inherent delays in the process as Solaar is just a regular program and might be delayed due to resource contention. |
|
Yeah thats fair enough, thanks for your working to get this implemented. The only issue I can still see is that the slider doesn't always work in Automatic mode, but I think thats to be expected. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2210