Inconsistent behavior in Android
Closed this issue · 10 comments
Alarm plugin version
3.0.14
Describe the bug
The plugin behaves a little bit differently across various Android contexts:
Android simulator
- On Kill Notification: it doesn't always show up. Sometimes it shows, sometimes it doesn't. It seems arbitrary as far as I can tell.
- App on foreground: stream receives the event of the alarm firing and notification shows up. All good!
- App on background: Notification shows. Tapping on it stops the sound and opens the app, but the stream doesn't receive any event, which means that the app cannot navigate to a screen that shows that an alarm has fired. In iOS, the stream does receive an event and the notification doesn't stop the sound. Only the app stops the sound, which is good I think, considering that snoozing is a popular feature.
- App killed: even with the app killed, the alarm still fires. One of my user uninstalled the app to avoid the alarms being fired because they felt they cannot control them. I don't think this is good. Delivering alarms is important, but as long as the on kill notification is there and the user understands that killing the app means no alarms, it's sufficient. It also offers parity with iOS which is important, I think.
Samsung Galaxy A33
- On Kill Notification: it works, but it doesn't use the app icon, title or description that is provided. This is problematic because my app is localized while the default plugin notification isn't. Weird! I hope this is just because of the closed testing thing, but I will test on more devices soon.
- App on foreground: stream receives the event of the alarm firing and notification shows up. All good!
- App on background: notification shows up, tapping on it opens the app and the stream receives an event. All good!
- App killed: the app is suddenly opened and the notification is shown. The sound doesn't loop and stops by itself after playing once.
To Reproduce
Just deploy and test
Expected behavior
I'm not sure what is supposed to be the ideal behavior of the plugin, as there seems to be some differences between iOS & Android. As far as I'm concerned, it should be something like this:
- On Kill Notification: must work consistently and use the app icon, title & message
- App on foreground: no need for the notification to show, but if it does, it's fine. The stream must receive an event and interacting with the notification must not stop the alarm. The alarm should only be stopped from the app itself since it's the one that scheduled it and might have its own process on how to do that.
- App on background: the notification must be displayed. Tapping on it must not stop the alarm. Instead, it must open the app and the stream must receive an event so that the app can update its UI and offer the user its own way of stopping the alarm.
- App killed: no alarms. It's really scary to know that you have killed an app and yet, it still might fire an alarm regardless of the Do Not Disturb state. This is parasitic behavior that makes the user feel as though they lost control.
Screenshots
No screenshots needed.
Device info
Android simulator, Samsung Galaxy A33... Will update with more devices.
Additional context
None
I have tested on another Android device. It's similar to the Galaxy A33.
Honor PCT
- On Kill Notification: it works but isn't customizable! It uses the default text and message, and a different icon: white with a red dot. Japanese flag? :)
- App on foreground: stream receives the event of the alarm firing and notification shows up. Works great!
- App on background: sometimes, tapping the notification opens the app with the stream having received the event and the sound still playing. Sometimes, tapping the notification opens the app but the stream hasn't received any event and the sound stops. Weird!
- App killed: the app is suddenly opened and the notification is shown. The sound doesn't loop and stops by itself after playing once. Same behavior as the Galaxy A33.
Hi @younesouhbi
Thank you so much for your feedback ! It's very helpful.
About stream issues, I recently released v3.1.0 with some fixes. Some users had issues with their stream due to conflicts with other packages they used, like Firebase Messaging. More details here: #110 #122.
About the notification icon, it's a known issue I need to fix: #54.
About the alarm that is stopped on notification tap, it's also a known issue: #177.
About the notification on kill that doesn't always shows, I will work on it, but in my opinion it's not really necessary because the alarm will ring even if the app was killed.
A lot of work needs to be done ! Unfortunately I'm mostly alone working on the plugin so it can take a while. I hope some developers will contribute someday so issues can be resolved faster.
About the notification icon issue, I just released version 3.1.1 with a potential fix. Let me know if it works for you.
I do want to help, however seeing that native Android development is new to me, I'm going to need time to understand top level concepts before being of any help.
One of my Android testers reported not being able to turn off the alarm at all. Tapping the notification didn't stop the sound, and when they got into the app, the stream didn't receive the event, which means that there were no screen to let them stop the alarm. So they had to restart their phone.
I would say that the plugin works pretty well in iOS, but in Android, its behavior is too inconsistent. I will be spending the next few days reading and trying to understand how the Android version works. If you can offer some general idea how the various classes work and interact, it would be great.
Thank you for all your work on this!
I'm trying to understand how the plugin works. This is the process that supposedly executes when "setAlarm" is called. Is this correct?
Hi @younesouhbi
Yes your diagram looks correct !
Furthermore, in the AlarmService, AlarmPlugin.eventSink is called to send the information to the Flutter side (listenToAlarmEvents in AndroidAlarm) to tell that the alarm started ringing and to add the event to the ringStream.
It's true that the Android behavior is inconsistent. It looks like every device manufacturer (Samsung, Xiaomi, ...) has different behaviors with the plugin.
Really don't hesitate if you have any other question on the code. I'll try to answer as quickly as possible. Thanks a lot for your willingness to contribute, it’s great !
After having studied the native Android plugin and learned a lot about Android development, I have found some potential bugs and hopefully corrected them. I'm interested in your thoughts regarding this. Here is a list of what I did:
- AlarmReceiver: before, the AlarmManager would call this receiver when an alarm is due. This receiver would start the AlarmService which would ring the alarm. It seems like this is a common pattern, but I like removing everything that can be removed. So, I removed this and the AlarmManager directly starts the AlarmService as a foreground service.
- Notification On Kill: if you think about it, this is not needed in Android at all. In Android, the AlarmManager handles the alarm regardless of whether the app is alive or not. However, if the app is killed when the AlarmService starts, the user will tap on the notification, which will open the app, and here, there is an issue on the Flutter side of the Android plugin: the subscription to the stream in AndroidAlarm.init is too late. It never catches the data sent by the AlarmService through the event sink.
- Companion object communication: this is used between AlarmPlugin and AlarmService. It seems to be a bad practice. Essentially, if I understood correctly, these are like static variables shared without any thread safety. It's very possible that the method isRinging gets called while the AlarmService is modifying the ringingAlarmIds or that they get modified right after. To avoid this, I added a BroadcastReceiver into AlarmPlugin (registers on attach, deregisters in detach) that listens for updates from AlarmPlugin and/or requests them.
- AudioService: only one alarm can really ring at any given time. I didn't see any need to have multiple ids here, so I made it only one single id. If a request to ring an alarm comes and there is already one ringing, if it's the same, ignore the request, if it's a different alarm, then stop the current before ringing the new one. Now, there is only one MediaPlayer and one Timer in AudioService.
- getCurrentlyRingingAlarmId: this is a new method added to the native plugin. It solves the issue I mentioned earlier. It's possible that the app is started through the user tapping the notification after an alarm started ringing. In this case, the stream will miss it, leading to a case reported by one of my testers: no way to stop a ringing alarm. Only a phone restart would do, which is a really bad user experience that will lead to an uninstall and probably a 1 start review. To solve this, the AndroidAlarm calls this method right after listening. If something is ringing, it gets the settings and sends them through the Alarm stream to the Flutter app. After changing the AndroidAlarm.init, the AlarmStorage needs to be init before the AndroidAlarm. It's simply a matter of changing the order.
I would like to continue refactoring things. For example merging VolumeService into AudioService since it's the same functionality. Renaming things (quite anal about how things are named :D). I also added a lot of documentation and comments. Moreover, ideally, it would be great if the Alarm plugin used static properties and methods sparingly, as they are evil. A singleton would be better, but such a rewrite is quite huge. Maybe if my app ends up being reasonably successful, so probably no :)
I will share the code after I finish everything and clean up the code. Now, I'll be moving to the iOS plugin to see if there is a way to reduce battery usage. I especially want to test an idea: while the upcoming alarm is more than 15 minutes away, only do background refreshes. Once it gets close, then play the audio to keep the app active in the background. Rinse and repeat. In all honesty, I have absolutely NO idea how background fetch works on iOS, for all I know, the plugin already does this, or it might be impossible to do. I simply won't know until I research, so might be a stupid idea.
Thank you again!
Hi @younesouhbi,
Thank you so much for your work. I'm aware that the codebase could be improved significantly. Unfortunately, sometimes I've had to prioritize releasing features and fixing issues over maintaining quality. Your improvements would be incredibly helpful, and I'm looking forward to your pull request! I'm confident that the alarm plugin will be successful if we work together to enhance it. It's a big relief to know that someone with your skills is willing to contribute to the native code. I started learning Kotlin and Swift through this plugin.
Regarding the iOS part of the plugin, I'm currently experimenting with battery consumption by testing various silent audio lengths. I’m also developing a method to play silent audio for just one second every ten seconds to minimize active playback while ensuring the app remains operational in the background. I'll keep you informed on this. It’s important to note that iOS background fetch intervals can vary significantly, especially when the system is handling other intensive tasks like gaming or downloads. The techniques I’m using, like silent audio and background fetch, are the result of multiple experiments to maintain app activity in the background, ensuring the alarm sounds precisely when scheduled, even days later.
Thanks again for everything, and please reach out if you have any questions!
@younesouhbi I'd be particularly interested in getCurrentlyRingingAlarmId
I've got an app using this package, and it behaves perfectly fine in the android emulator, but on two Moto android devices, it's impossible to dismiss the alarms unless I kill the app. If I can getCurrentlyRingingAlarmId when the app is opened, I could at least stop the alarm there
I'm closing this issue because in the meantime version 4.0.0 was released including a significant refactoring of the code and many bug fixes. Feel free to reopen if needed !