Skip to content

Conversation

@davidrapan
Copy link
Contributor

@davidrapan davidrapan commented Oct 31, 2025

Proposed change

Use ignore_variance to compare new value against last one w/ 1 minute invariance, so we fix log spam:
starlink_last_boot_time_spam

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:
  • Link to developer documentation pull request:
  • Link to frontend pull request:

Checklist

  • I understand the code I am submitting and can explain how it works.
  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@home-assistant
Copy link

Hey there @boswelja, mind taking a look at this pull request as it has been labeled with an integration (starlink) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of starlink can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign starlink Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@davidrapan davidrapan force-pushed the dev-starlink_last_restart branch from 69308fb to 513cc2a Compare October 31, 2025 23:44
@davidrapan davidrapan marked this pull request as draft November 1, 2025 09:56
@davidrapan davidrapan changed the title Fixes Starlink's ever updating last boot time Fixes Starlink's ever updating uptime Nov 1, 2025
@davidrapan davidrapan force-pushed the dev-starlink_last_restart branch from b924242 to 118dbab Compare November 1, 2025 10:50
@davidrapan davidrapan changed the title Fixes Starlink's ever updating uptime Fix Starlink's ever updating uptime Nov 1, 2025
@davidrapan davidrapan marked this pull request as ready for review November 1, 2025 11:26
@davidrapan davidrapan requested a review from boswelja November 1, 2025 14:00
@davidrapan davidrapan force-pushed the dev-starlink_last_restart branch from a7bf312 to c1fa9b9 Compare November 3, 2025 15:15
Copy link
Member

@joostlek joostlek left a comment

Choose a reason for hiding this comment

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

Instead, look at a solution like homewizard did for a similar problem
#150680

@home-assistant home-assistant bot marked this pull request as draft November 25, 2025 20:14
@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

Comment on lines 125 to 151
async def test_last_restart_state(hass: HomeAssistant) -> None:
"""Test Starlink last restart state."""
entry = MockConfigEntry(
domain=DOMAIN,
data={CONF_IP_ADDRESS: "1.2.3.4:0000"},
)
entity_id = "sensor.starlink_last_restart"

with (
LOCATION_DATA_SUCCESS_PATCHER,
SLEEP_DATA_SUCCESS_PATCHER,
STATUS_DATA_SUCCESS_PATCHER,
HISTORY_STATS_SUCCESS_PATCHER,
patch(
"homeassistant.components.starlink.sensor.now",
return_value=datetime.fromisoformat("2025-10-22T13:31:29+00:00"),
),
):
entry.add_to_hass(hass)

await hass.config_entries.async_setup(entry.entry_id)
await hass.async_block_till_done()

assert entry.runtime_data
assert entry.runtime_data.data
assert entry.runtime_data.data.status["uptime"] == 804138

Copy link
Member

Choose a reason for hiding this comment

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

Oh right, forgot to tell about the tests.

Ideally we don't touch internals like runtime_data. Also, we shouldn't patch now, and instead use the freezer. I have been playing around this for a little bit to refactor this test to use it, but I didn't have much luck, but that'd be nice if we could have that

Copy link
Contributor Author

@davidrapan davidrapan Nov 25, 2025

Choose a reason for hiding this comment

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

Yeah, I kinda wasn't able to make the tests work so this was some sort of a last resort. :) But maybe now we don't need to test it as a sensor and simply test the method?

BTW, I wasn't aware of the ignore_variance wrapper, this is certainly better. 👍

Copy link
Member

Choose a reason for hiding this comment

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

In a way I think having the test test the sensor is best as that's eventually what the user will see, so testing only the method makes it a bit more fragile

Copy link
Contributor Author

@davidrapan davidrapan Nov 27, 2025

Choose a reason for hiding this comment

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

678c4dd is the only way I managed to get it working.

Signed-off-by: David Rapan <[email protected]>
@davidrapan davidrapan marked this pull request as ready for review November 26, 2025 09:35
@home-assistant home-assistant bot requested a review from joostlek November 26, 2025 09:35
Signed-off-by: David Rapan <[email protected]>
@joostlek joostlek added this to the 2025.12.1 milestone Dec 4, 2025
@joostlek joostlek merged commit 845c9ee into home-assistant:dev Dec 4, 2025
36 checks passed
@davidrapan davidrapan deleted the dev-starlink_last_restart branch December 4, 2025 19:35
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants