android icon indicating copy to clipboard operation
android copied to clipboard

Fix sensorDao not initialized error after geocoded update

Open dshokouhi opened this issue 3 years ago • 2 comments

Summary

Fixes the following error, by passing in the sensorDao variable instead of relying on the lateinit modifier. I was able to reproduce the error by force stopping the app and opening it again. Must be an edge case where we update location before we update sensors completely and initialize the variable.

[08-06 11:26:33.414 26944:11135 E/LocBroadcastReceiver]
Could not update location.
kotlin.UninitializedPropertyAccessException: lateinit property sensorDao has not been initialized
	at io.homeassistant.companion.android.common.sensors.SensorReceiverBase.getSensorDao(SensorReceiverBase.kt:57)
	at io.homeassistant.companion.android.common.sensors.SensorReceiverBase.updateSensor(SensorReceiverBase.kt:340)
	at io.homeassistant.companion.android.common.sensors.SensorReceiverBase$updateSensor$1.invokeSuspend(Unknown Source:19)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
	at kotlinx.coroutines.internal.LimitedDispatcher.run(LimitedDispatcher.kt:42)
	at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:95)
	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:570)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:677)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:664)

Screenshots

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

Ref: https://community.home-assistant.io/t/location-updating-doesnt-work-very-serious-problem-for-my-automations/437206/88?u=dshokouhi

dshokouhi avatar Aug 07 '22 16:08 dshokouhi

Must be an edge case where we update location before we update sensors completely and initialize the variable.

I have been thinking about this and don't quite follow. Shouldn't Hilt initialize this as soon as you create the class? There is no sensorDao = ... in the class.

jpelgrom avatar Aug 09 '22 17:08 jpelgrom

Must be an edge case where we update location before we update sensors completely and initialize the variable.

I have been thinking about this and don't quite follow. Shouldn't Hilt initialize this as soon as you create the class? There is no sensorDao = ... in the class.

that I am not sure, I assume when I did the force stop that the update tried to send before the sensor worker kicked in. That might explain why I didnt see it during initial testing?

dshokouhi avatar Aug 09 '22 17:08 dshokouhi

@dshokouhi Now that there has been a stable release with these changes, how often does this appear in Sentry?

jpelgrom avatar Aug 30 '22 15:08 jpelgrom

@dshokouhi Now that there has been a stable release with these changes, how often does this appear in Sentry?

It's actually a handled exception so it's not being sent to sentry.

dshokouhi avatar Sep 01 '22 13:09 dshokouhi

Decided to look at this in more detail, and it turns out that Hilt does injection for BroadcastReceivers in the onReceive function. Because the app creates the class but doesn't call onReceive, the fields are never injected.

It is relatively easy to reproduce by adding a custom intent action to update the sensor in the LocationSensorManager, and immediately calling SensorReceiver().updateSensor(...), which skips the need for location updates. Then send a broadcast with the action to trigger it, for example: adb shell am broadcast -a io.homeassistant.companion.android.test.geocoded io.homeassistant.companion.android.debug.

While this change will prevent the exception, it isn't pretty. It'd be better to add another action to SensorReceiver's intent filter to update an individual sensor so that onReceive is used 🙂

jpelgrom avatar Sep 01 '22 18:09 jpelgrom

Ok that makes sense and explains why it only happens in certain cases. So instead of calling the method directly we need to send a intent to process the data. I'll get around to that in next couple of days since I'm traveling today.

Thanks for looking into this further!

dshokouhi avatar Sep 01 '22 19:09 dshokouhi

Ok cleaned up the update method a bit to take less information as not all the data was easy to send over in the intent. Really only need the BasicSensor to handle the individual sensor update.

dshokouhi avatar Sep 03 '22 16:09 dshokouhi