Add start workers in LaunchActivity

[TimoPtr]

Summary

Replicate the start workers from the WebViewActivity within the LaunchActivity. It does replicate exactly the existing behavior, it is not gated behind WipFeature because it just triggers a second update of the sensor which is acceptable.

Checklist

• New or updated tests have been added to cover the changes following the testing guidelines.
• The code follows the project's code style and best_practices.
• The changes have been thoroughly tested, and edge cases have been considered.
• Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Any other notes

[TimoPtr]

@jpelgrom I'm struggling here to find a nice way to work with this and I need more context.

• I don't want to use any Context in the ViewModel
• ViewModel should stay unaware of the onResume/onStart IMO if possible

From my point of view it is also not the responsibility of the Screen to start the workers.

I can modify the start function to not take any Context by injecting the WorkerManager in the ViewModel, but I wonder if the logic of starting in onResume is mandatory or if it can only be made one time when the VM is created?

I'm curious about your opinion here.

[github-actions]

Test Results

  232 files    240 suites   9m 53s ⏱️
1 787 tests 1 786 ✅ 0 💤 1 ❌
1 858 runs  1 856 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 6af7901.

♻️ This comment has been updated with latest results.

[jpelgrom]

@jpelgrom I'm struggling here to find a nice way to work with this and I need more context.

* I don't want to use any `Context` in the ViewModel

* ViewModel should stay unaware of the onResume/onStart IMO if possible

From my point of view it is also not the responsibility of the Screen to start the workers.

I can modify the start function to not take any Context by injecting the WorkerManager in the ViewModel, but I wonder if the logic of starting in onResume is mandatory or if it can only be made one time when the VM is created?

I'm curious about your opinion here.

I think the goal the current behavior is trying to achieve is simply more updates for sensors / a way to kickstart any workers that the system may have suspended, as it is unlikely they will be delayed when the app is actively being used. That means tying it to the entrypoint for users -> WebViewActivity. (@dshokouhi might know more history here.)

Following that logic, it isn't really about the specific screen but rather the entire app being opened or hidden (lifecycle).

I kind of understand what you're trying to do with the ViewModel taking no context, but could you spell it out? There are still tons of Android-specific functions that exist and need context so I'm not sure it can be completely avoided forever, and to do everything in Compose where you have a context feels like misuse.

ViewModel doesn't need to know about onResume/onStart but we could want functions called on those lifecycle events. If multiple things need to happen on onStart I'm not opposed to simply creating a function named onStart in the ViewModel to make it easier to manage 🤷.

[dshokouhi]

Yes we do have an app importance sensor that needs to update if the app is in foreground or what not. That's why the on pause and on resume calls are there. It should be on the settings activity as well iirc

[TimoPtr]

@jpelgrom we could potentially move the logic within the ViewModel but I'm not sure if it relevant here, it would force us to pass the context or change how we start the workers.

[Copilot]

Pull request overview

This PR updates LaunchActivity (the app’s navigation entry Activity) to start the same background workers that are currently started from WebViewActivity, so sensor updates and the persistent WebSocket worker are initiated when the app resumes.

Changes:

• Start SensorWorker and schedule WebsocketManager from LaunchActivity.onResume()
• Trigger an immediate sensor refresh via SensorReceiver.updateAllSensors() from LaunchActivity.onPause()
• Expand LaunchActivity KDoc to describe these lifecycle behaviors

[jpelgrom]

Moving it into LaunchActivity means that, at least as long as the WebViewActivity is used, these two updates + the update in WebViewActivity launch trigger very shortly after another because you transition in and out of LaunchActivity almost immediately. Maybe put these behind the feature flag as well?

[jpelgrom]

This makes no sense to me, and I wouldn't be surprised if it turns out to be flaky. The documentation mentions (including emphasis!):

Started state for a LifecycleOwner. For an android.app.Activity, this state is reached in two cases:

• after android.app.Activity.onStart call;
• right before android.app.Activity.onPause call.

[TimoPtr]

It's for the test library

for me when calling moveToState with STARTED it does invoke pauseActivity

I didn't see any flakiness on this test, I did run it multiple times without any issue.

[jpelgrom]

Love the Google inconsistency... /s

In that case OK, no objection and it seems like the right way to test this.