-
-
Notifications
You must be signed in to change notification settings - Fork 524
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
Add mouse DPI notifier #2238
base: master
Are you sure you want to change the base?
Add mouse DPI notifier #2238
Conversation
Found an issue with the polling of
It seems that when the mouse receiver is connected, and the mouse cannot be reached through the receiver, this error/warning is logged because the command times out? Preliminary thoughts, anyway. Will investigate and bring out of draft once resolved. |
Interesting idea. What is quite unfortunate for this and any other kind of 'notifier' is that I'm not aware of any mechanism where the device tells us actively that something has changed as opposed to us querying the value continuously. I'm also wondering if we could limit this code to devices with DPI buttons or something but I don't think we have this information available at all, because we're just wasting power for devices where you can't change the DPI on the device anyways. But for sure for this I'd disable the notifier by default and interested users can enable it if they want to. |
The first case, perhaps the easiest case, is when the user changes the DPI from an application like Polychromatic. They see the DPI they are setting.
For the second case, a physical button being pressed to change the DPI, we may be able to leverage the raw mouse event handling done in the mouse driver. One example use case (in code) for this raw event processing is for wheel tilt edge detection for various devices (screenshot below). This is one place where we have the opportunity to "act" upon the pressing of the DPI button. Here's a working example of acting upon the DPI button being clicked on (screenshot below). I don't have any solid ideas yet for any specific actions to take, but this is where my archeological dig has landed me thus far 😂. Another opportunity for detecting changes would be detecting changes to the "dpi device file" from within openrazer-daemon; when
If there are no ways to identify devices with changing DPI's via code, then we could selectively enable this for those devices that we do know ahead of time; ie: Razer Pro Click. If people come asking for support for a device that is not currently supported, it should be relatively simple to add such support, like is done with other device specific code/support.
I agree we should leave it disabled by default, like you suggested below. This should solve the running unnecessarily in the general case. |
Okay.. So some time ago I booted up a Windows VM with Synapse and also saw that they have this popup when you change the DPI using the buttons so having this functionality is probably desired by some people. Assuming we will switch devices with DPI buttons to "driver mode" (from "device mode") then we can handle the button events ourselves and then we obviously know when the button is pressed since we need to handle it ourselves anyways. But I think doing that without regressing some functionality on some random mice for users is not possible yet, so we probably want to have something for the short-term if somebody really wants this... So.. to the code: For starters what I don't quite get is why you built the loop the way you did, every 0.1 seconds in any case you call |
@justintime4tea Any update on this? |
There are no updates as of now, but I will take a look today and try to make some sort of progress. Thanks for the reminder! 🙏 |
I agree wholeheartedly. In the future, if mice are in "driver" mode, we won't have to poll anything and will be much cleaner.
As far as the 0.1 sleep and coding style/structure, I just copied BatteryNotifier and only changed the core logic for determining when to notify and what to notify, keeping the existing structures and style in-place. I have made three updates:
|
PS: I'm "battle testing" this by running with a 500ms frequency and leaving it run for quite some time to make sure these 500ms back-to-back dbus calls don't cause issues. My system isn't a great test for this, though, because it is ridiculously beefy... |
Two more considerations:
|
|
||
dpi_level = str(dpi_level_x) | ||
if dpi_level_x != dpi_level_y: | ||
dpi_level = "[x:{0}, y:{1}]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this need some .format()
?
|
||
def close(self): | ||
""" | ||
Close the manager, stop ripple thread |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ripple -> DPI notifier
mouse_dpi_notifier = False | ||
|
||
# Mouse DPI notification frequency [ms] (0 to disable) | ||
# WARNING: The openrazer-daemon will poll an openrazer DBUS service at this frequency |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"[...] will poll the mouse at this frequency"
Also, why have both = 0
here to disable but also the boolean? Can we get rid of this boolean for example?
|
||
# Mouse DPI notification frequency [ms] (0 to disable) | ||
# WARNING: The openrazer-daemon will poll an openrazer DBUS service at this frequency | ||
# WARNING: The openrazer dbus service uses libdbus which is not out-of-memory safe- YMMV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate what this means for the end user?
Also when you think this is ready, please set as Ready on GitHub (so it's not a Draft anymore) |
It is the intent of this PR to propose a "notifier" similar to that of the existing
BatteryNotifier
.It would be more ideal to create a general purpose "notifier" that could be used to support both cases; battery and mouse DPI notifications. I hadn't the time to take this on, would like an immediate solution to know what my mouse DPI is when changing it via physical button, and felt this might be a good way to start a conversation.
Also, it would be ideal if notifications, be them for the battery or the mouse DPI, were triggered in response to the battery level or mouse DPI changing, through some eventing mechanism, rather than short-polling. I have no idea the impact of making the
get_dpi_xy
oravailable_dpi
DBUS calls every 500ms (1000ms for the case of battery).