Skip to content

Fix settings update issue by making SensorSetting immutable - #7117

Merged
TimoPtr merged 3 commits into
mainfrom
fix/settings_update
Jul 3, 2026
Merged

Fix settings update issue by making SensorSetting immutable#7117
TimoPtr merged 3 commits into
mainfrom
fix/settings_update

Conversation

@TimoPtr

@TimoPtr TimoPtr commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

The sensor settings update where not showing as per @jpelgrom found out #7041 (comment) with the recent changes we've made.

I've fixed it by making SensorSetting immutable and exposing StateFlow for it. The issue was that now that the sensor is immutable the way we've updated the Settings it was not triggering a change on the compose side because the object itself was still the same reference.

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

Based on #7041

@TimoPtr
TimoPtr requested a review from jpelgrom July 2, 2026 09:58
@TimoPtr
TimoPtr force-pushed the feature/sensor_immutable branch from 5b110af to 8c3a9c2 Compare July 2, 2026 10:10
@TimoPtr
TimoPtr force-pushed the fix/settings_update branch from 302033e to 442927a Compare July 2, 2026 10:11
@TimoPtr
TimoPtr force-pushed the feature/sensor_immutable branch from 8c3a9c2 to c870331 Compare July 2, 2026 10:48
@TimoPtr
TimoPtr force-pushed the fix/settings_update branch from 442927a to 7eaa35d Compare July 2, 2026 10:48
@TimoPtr
TimoPtr force-pushed the feature/sensor_immutable branch from c870331 to 1ae01d7 Compare July 2, 2026 11:18
@TimoPtr
TimoPtr force-pushed the fix/settings_update branch from 7eaa35d to c26cb81 Compare July 2, 2026 11:18
Base automatically changed from feature/sensor_immutable to main July 2, 2026 11:54
@TimoPtr
TimoPtr force-pushed the fix/settings_update branch from c26cb81 to 1944254 Compare July 2, 2026 11:54

@jpelgrom jpelgrom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and this correctly fixes the issue. Thanks for the fix!

private const val SENSOR_SETTING_TRANS_KEY_PREFIX = "sensor_setting_"

// Keep the database-backed flows hot briefly across config changes before stopping collection.
private const val STOP_TIMEOUT_MILLIS = 5_000L

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use a duration here instead of millis which should be preferred. 5 seconds also feels very long for configuration changes.

(This also needs updated imports so simply accepting suggestion won't work)

Suggested change
private const val STOP_TIMEOUT_MILLIS = 5_000L
private val STOP_TIMEOUT = 1.seconds

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ashamed ... I used claude for this and I didn't even see that it was not following the convention I wrote... 🥹

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also made it 500ms like we had in other places.

Copilot AI review requested due to automatic review settings July 3, 2026 06:47
@TimoPtr
TimoPtr force-pushed the fix/settings_update branch from 1944254 to ce2b954 Compare July 3, 2026 06:47
@TimoPtr
TimoPtr enabled auto-merge (squash) July 3, 2026 06:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a regression where sensor setting changes were not reflected in the Compose UI after recent immutability changes, by making SensorSetting immutable and switching the sensor detail screen to consume database-backed StateFlows.

Changes:

  • Make SensorSetting.value immutable (varval)
  • Refactor SensorDetailViewModel to expose StateFlow for sensors and sensor settings (via stateIn)
  • Update SensorDetailView to collect these flows using collectAsStateWithLifecycle and update dialog submission to use copy(...)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
common/src/main/kotlin/io/homeassistant/companion/android/database/sensor/SensorSetting.kt Makes sensor setting values immutable to ensure state updates propagate via new instances
app/src/main/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt Collects StateFlow values in Compose and updates setting submission to use immutable copies
app/src/main/kotlin/io/homeassistant/companion/android/settings/sensor/SensorDetailViewModel.kt Exposes sensors/settings as StateFlow backed by database flows using stateIn

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@TimoPtr
TimoPtr merged commit 6a601fe into main Jul 3, 2026
24 checks passed
@TimoPtr
TimoPtr deleted the fix/settings_update branch July 3, 2026 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants