Fix left notification when high accuracy mode was disabled #1366
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
Some points which may fix this issue * Make notification objects static * Use ServiceCompat for stopForeground * Additionally call cancel to remove notification
dshokouhi
reviewed
Feb 20, 2021
| isRunning = false | ||
| Log.d(TAG, "High accuracy location service stopped") | ||
|
|
||
| // Remove notification again. Sometimes stopForeground(true) is not enough. Just to be sure | ||
| notificationManagerCompat.cancel(notificationId) |
There was a problem hiding this comment.
I noticed I had to add similar logic in #1265 we just always have to cancel. I think its due to the app being silently killed in the background. I found a good way to test it is once you have the notification posted to remove the app from the recent apps list which isn't like a force stop but causes the variables to reset. I think thats the case and reason for just always removing at the end.
There was a problem hiding this comment.
Thanks for help 👍 but also with force close app i couldn't reproduce the issue. It is weird. I can only see this issue when i'm using this in my car. And also not every time :/ Most of the time it does work.
JBassett
approved these changes
Feb 26, 2021
Alfiegerner
pushed a commit
to Alfiegerner/android
that referenced
this pull request
Feb 28, 2021
…stant#1366) Some points which may fix this issue * Make notification objects static * Use ServiceCompat for stopForeground * Additionally call cancel to remove notification
JBassett
added a commit
that referenced
this pull request
Mar 1, 2021
…ter, improve robustness (#1384) * First cut add BLE beacon as new sensor. * Lint issues, also using hardcoded strings for state. * Fixes and small tidies following review. * Add command_ble_transmitter to MessagingService.kt, to control BLE Transmitter from HA notifications. * Make description more specific for settings. * Return if sensor not enabled * Spaced. * Command turn on for transmitter should stop it first, as state could be incorrect on Bluetooth being disabled. * Don't attempt to transmit if BT is off, and include that information in the state. * Update app/src/main/java/io/homeassistant/companion/android/sensors/BluetoothSensorManager.kt Co-authored-by: Justin Bassett <bassett.justint@gmail.com> * Add 2 new settings: - Power transmitter values between high and ultra low. Defaults to ultra low, should help with Battery concerns - A setting which allows the sensor to be turned on by default when the Enable all toggle is turned on. This is off by default, users have to actively seek out this sensor to turn it on. * Add battery warning to description. * Fix build - stray merge comma deletion. * Fix small description text error. * Fix left notification when high accuracy mode was disabled (#1366) Some points which may fix this issue * Make notification objects static * Use ServiceCompat for stopForeground * Additionally call cancel to remove notification * Bump some app dependencies (#1374) * Add BLE ibeacon transmitter as sensor to allow room presence. (#1343) * First cut add BLE beacon as new sensor. * Lint issues, also using hardcoded strings for state. * Fixes and small tidies following review. * Add command_ble_transmitter to MessagingService.kt, to control BLE Transmitter from HA notifications. * Make description more specific for settings. * Return if sensor not enabled * Spaced. * Command turn on for transmitter should stop it first, as state could be incorrect on Bluetooth being disabled. * Don't attempt to transmit if BT is off, and include that information in the state. * Update app/src/main/java/io/homeassistant/companion/android/sensors/BluetoothSensorManager.kt Co-authored-by: Justin Bassett <bassett.justint@gmail.com> * Add 2 new settings: - Power transmitter values between high and ultra low. Defaults to ultra low, should help with Battery concerns - A setting which allows the sensor to be turned on by default when the Enable all toggle is turned on. This is off by default, users have to actively seek out this sensor to turn it on. * Add battery warning to description. * Fix build - stray merge comma deletion. * Fix small description text error. Co-authored-by: Justin Bassett <bassett.justint@gmail.com> * And fix service fields being added too many times (#1377) * Improve robustness when BT is turned on and off. Also, tidy code, keep sense of when device parameters have changed in one place. Co-authored-by: Justin Bassett <bassett.justint@gmail.com> Co-authored-by: chriss158 <edgi@arcor.de> Co-authored-by: Daniel Shokouhi <dshokouhi@gmail.com> Co-authored-by: Bram Kragten <mail@bramkragten.nl>
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.
Summary
Again i had the high accuracy notification still visible, even though the high accuracy mode was disabled due to not connected BT devices. This happens not every time and i can't reproduce this.
This PR hopefully fixes the problem.
Some points which may fix this issue
Screenshots
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#
Any other notes