enviroCar / enviroCar-app

enviroCar Android Application
https://envirocar.org
GNU General Public License v3.0
88 stars 154 forks source link

App crash when disabling auto connect #877

Closed SebaDro closed 1 year ago

SebaDro commented 2 years ago

Description The app crashes when you try to deactivate the "Auto Connect" setting.

Branches master, develop

How to reproduce Go to the settings screen, enable "Auto Connect" and try to disable it again.

How to fix TBD

cdhiraj40 commented 2 years ago

@SebaDro sir is there any more pre-requisite to reproduce this? I am unable to reproduce it, maybe device specific?

pree-T commented 2 years ago

Is this issue open? May I work on it?

SebaDro commented 2 years ago

@SebaDro sir is there any more pre-requisite to reproduce this? I am unable to reproduce it, maybe device specific?

@cdhiraj40: Yes, actually the app only crashes if Bluetooth is not enabled. I checked it with Bluetooth enabled and everything works fine. Can you reproduce it when you try to disbale "Auto Connect" with disabled Blueooth?

cdhiraj40 commented 2 years ago

@cdhiraj40: Yes, actually the app only crashes if Bluetooth is not enabled. I checked it with Bluetooth enabled and everything works fine. Can you reproduce it when you try to disbale "Auto Connect" with disabled Blueooth?

@SebaDro I can confirm this as a bug, the app crashes every time I try to disable Auto Connect. I would notify here, as soon as I get any information on this :) I have noticed it crashing even if we don't disable it. To produce: Enable Auto Connect (no Bluetooth) and wait for 10-20 secs. (it happens every time settings screen opens too and app crashes after 10-20 secs) is this behavior happening with you too sir?

SebaDro commented 2 years ago

Actually, it only happens when I try to disable it again with Bluetooth disabled. It seems to be an issue regarding the AutoRecordingService.

Do you want to work on it? If so, please, analyze the problem first and come up with an approach for fixing it, just before you are starting the implementation.

cdhiraj40 commented 2 years ago

It seems to be an issue regarding the AutoRecordingService.

Yeah, I got to know the same, its crashing when the stopService method is called. I have also noticed something interesting that, if Bluetooth is disabled and location is on then the app doesn't crash. I am checking why this behavior is happening...

Do you want to work on it? If so, please, analyze the problem first and come up with an approach for fixing it, just before you are starting the implementation.

I would sure like to work on this. Sure I will talk about my approach before implementing it, Thanks

cdhiraj40 commented 2 years ago

@SebaDro sir the reason for its crashing seems to be '' Reason: Context.startForegroundService() did not then call Service.startForeground() '' I checked about it here I think we have to take care of devices having API level >= 26(After Android 8.0) while creating a new service.
official documentation for Android 8.0 does say that we have to call startForeground() in our service within 5 seconds of creating a new one. And after checking in AutoRecordingService I got to know that we are never calling startForeground, so that it crashes for below API level too (but here I don't think so we have restrictions of 5 seconds, also I am not sure about the minimum API Level, I checked for API 24 and it was crashing)

I think now we have one option only, which is to pass startForeground function with notification ID and notification. What do you think sir?

cdhiraj40 commented 2 years ago

@SebaDro sir I tested the above approach by adding this code block with a function in onCreate of AutoRecordingService.java.

        if (Build.VERSION.SDK_INT >= 26) {

            String CHANNEL_ID = "channel ID";
            String CHANNEL_NAME = "Channel Name";

            NotificationChannel channel = new NotificationChannel(CHANNEL_ID,
                    CHANNEL_NAME, NotificationManager.IMPORTANCE_NONE);
            ((NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE)).createNotificationChannel(channel);

            Notification notification = new NotificationCompat.Builder(this, CHANNEL_ID)
                    .setCategory(Notification.CATEGORY_SERVICE).setSmallIcon(R.drawable.img_envirocar_logo).setPriority(PRIORITY_MIN).build();

            startForeground(101, notification);
        }
    }

And it seems to be working. I didn't face any crashes with this approach. What do you think sir?

SebaDro commented 2 years ago

It looks good to me. If want, you can create a pull request for it. But please, choose a senseful channel ID and name ;-) In addition, consider the already existing classes within https://github.com/enviroCar/enviroCar-app/tree/master/org.envirocar.app/src/org/envirocar/app/notifications. It would be more consistent to integrate your solutions in there.

cdhiraj40 commented 2 years ago

@SebaDro sure sir will do. Thanks!

cdhiraj40 commented 2 years ago

@SebaDro sir I forgot to mention but startForeground() is a method of service, so a Service has to call the method in itself. I think we have to call this function inside AutoRecordingService.java class's onCreate only? or is there a way to use the method here? If yes then please let me know the approach sir :)

cdhiraj40 commented 2 years ago

@SebaDro sir I forgot to mention but startForeground() is a method of service, so a Service has to call the method in itself. I think we have to call this function inside AutoRecordingService.java class's onCreate only? or is there a way to use the method here? If yes then please let me know the approach sir :)

@SebaDro sir any thoughts on this?

SebaDro commented 1 year ago

Fixed with 4c0e6edc.