Skip to content

Conversation

@jpcunningh
Copy link
Collaborator

I've tested and still receive pump battery low alarms with this code removed.

@sulkaharo
Copy link
Member

Did you check the git history of how these landed into the codebase?

@PieterGit
Copy link
Contributor

PieterGit commented Jan 20, 2019

@jpcunningh @sulkaharo
I did a git history check. I added it in the final part of the 0.10.3 release. We had some issues with the speech plugin (solved now). It was added with commit 1769f2d#diff-d878eabd64038e07845b240469aa46ca and it was part of #3709

From that issue:
I had serious problems with Warning, Pump Battery Low alarm.
It was is audible, but could not be surpressed or disabled in the UI. I started adding the UI for disabling it, but it does not seem to work yet. There is some strange issue with these pump alarms. Temporary workaround is PUMP_ENABLE_ALERTS=false, but of course PUMP_ENABLE_ALERTS=true should not lead to alarms that cannot be stopped and cannot be disabled in the UI.

I just had an empty battery, and currently it seems the battery alarm does not respect the setting in the UI (the alarm for low battery was audible with the setting "Pump Battery Low Alarm" enabled or disabled)

@PieterGit
Copy link
Contributor

PieterGit commented Feb 2, 2019

@jpcunningh @sulkaharo do you have any idea why the alarm-pumpbatterylow-browser does not work. I rather fix it that it works, than remove it. Agree with postponing this to 0.12?

@PieterGit PieterGit added this to the 0.12.0 milestone Feb 2, 2019
@jpcunningh
Copy link
Collaborator Author

@PieterGit, I have no problem with postponing.

We haven't experienced any of the symptoms you described. Do you see the symptoms on different browsers?

@jpcunningh jpcunningh closed this Feb 6, 2020
@jpcunningh jpcunningh deleted the remove-unused-client-setting branch February 6, 2020 16:05
@bebergamota
Copy link

bebergamota commented Feb 6, 2020 via email

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.

4 participants