-
Notifications
You must be signed in to change notification settings - Fork 12
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
Do not export alarm properties on D-Bus #104
Conversation
@tari01: can you please improve the commit message "Do not export/test alarm properties on D-Bus" and explain why you think this to be the approach to go? In 6 months from now, that one-liner will let us frown and scratch our bald heads. Thanks! |
This commit should resolve the alarm volume/vibration issue reported in Lomiri: https://gitlab.com/ubports/development/apps/lomiri-clock-app/-/issues/180 and upstream: AyatanaIndicators#103
5d99f27
to
84d5e75
Compare
Amended. Review, but do not merge until @JamiKettunen confirms this actually solves the problem. |
Hm I've not tested anything but wouldn't it be better to wait until UT 22.04 dev starts before merging this? Also does it always read the GSettings already every time when sounding alarm etc since after this it won't be possible to change them via D-Bus anymore like the Clock app currently does when messing with the settings? Or I suppose rather is the internal state of the indicator for those props updated when changes to the GSettings happen? Based on the issue of D-Bus props being unaccessible (due to AppArmor) that doesn't look to be the case currently |
Yes. The indicator tracks the GSettings internally and updates the variables accordingly. Nothing else is needed there. This change (supposedly) only prevents the indicator from exporting those values to D-Bus to avoid the clash resulting from Lomiri already doing the same. This should definitely be tested before merging. |
@JamiKettunen please test this, if possible. |
@JamiKettunen Is this PR still required / needed / valid? @tari01 Feedback? I will close this PR now, please reopen if you want to pick up the thread again. |
fixes #103
@JamiKettunen: Can I have your feedback on this, please?