Skip to content

Mark dynamic-devices as exempt in Renault IQS#143450

Closed
epenet wants to merge 2 commits into
devfrom
epenet-20250422-1449
Closed

Mark dynamic-devices as exempt in Renault IQS#143450
epenet wants to merge 2 commits into
devfrom
epenet-20250422-1449

Conversation

@epenet
Copy link
Copy Markdown
Contributor

@epenet epenet commented Apr 22, 2025

Proposed change

SSIA

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

  • 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.

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:

Comment on lines +50 to +52
dynamic-devices:
status: exempt
comment: Adding/removing a vehicle is a rare occurrence
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem useful to trigger a lookup every x minutes for new vehicles - we'd be using up valuable hits and renault is already rate-limited per account.

It's more likely that if someone adds a new car they will just reload the integration

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See also: #142964

Comment thread homeassistant/components/renault/quality_scale.yaml Outdated
Comment on lines +53 to +54
Adding/removing a vehicle is a rare occurrence, and we don't want to
waste valuable hits on Renault API, which is rate-limited
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.

This seems like odd reasoning to me: A rare occurrence + worrying about rate-limiting.

If I might suggest an alternative, could you trigger a config entry reload if the change is detected?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is no way to know without making a specific API "list vehicles" request

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@frenck are you suggesting that I should make an API call at regular interval to see if there are new vehicles?
If we are making a "list vehicles" call every few minutes we would need to reduce the interval for more important data calls.

I guess we could make a call every 24h, but seems a lot of work for very little benefit.

@home-assistant home-assistant Bot marked this pull request as draft April 29, 2025 17:27
@home-assistant
Copy link
Copy Markdown
Contributor

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.

@github-actions
Copy link
Copy Markdown

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes.
Thank you for your contribution!

@github-actions github-actions Bot added the stale label Jun 29, 2025
@epenet
Copy link
Copy Markdown
Contributor Author

epenet commented Jun 29, 2025

Not stale

@github-actions github-actions Bot removed the stale label Jun 29, 2025
@github-actions
Copy link
Copy Markdown

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes.
Thank you for your contribution!

@github-actions github-actions Bot added the stale label Aug 28, 2025
@epenet epenet force-pushed the epenet-20250422-1449 branch from ae3c892 to 052049d Compare August 28, 2025 09:17
@epenet
Copy link
Copy Markdown
Contributor Author

epenet commented Aug 28, 2025

Not stale

@github-actions github-actions Bot removed the stale label Aug 28, 2025
@epenet
Copy link
Copy Markdown
Contributor Author

epenet commented Sep 9, 2025

Waiting for resolution of home-assistant/developers.home-assistant#2662

@emontnemery
Copy link
Copy Markdown
Contributor

We discussed this in the core meeting, and the conclusion is that it's OK to reload the integration when detecting a rare occurrence, but not OK to not detect it at all.

@github-actions github-actions Bot locked and limited conversation to collaborators Sep 13, 2025
@epenet epenet deleted the epenet-20250422-1449 branch March 5, 2026 21:03
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.

3 participants