Add config flow to nederlandse_spoorwegen #148027
Conversation
…chpaul/core into feature/improving_NS_integration
…tion Feature/improving ns integration
…tion Feature/improving ns integration
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
There was a problem hiding this comment.
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
home-assistant
bot
added
cla-needed
code-quality
config-flow
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Oh and please don't merge We generally only do that when it has been out of sync for a LONG time, or when we know it will break, or for other reasons |
…eData for runtime data management, enhancing structure and readability. Added comprehensive subentry flow tests for route management, including validation, error handling, and successful route creation scenarios. Improved test coverage for options flow and reconfiguration processes, ensuring robust handling of user inputs and station data.
…chpaul/core into feature/improving_NS_integration
…oorwegen integration
…ation - Introduced , , and to analyze device and entity registries post-migration. - Implemented JSON error handling tests in to ensure graceful handling of JSON parsing errors. - Updated to remove obsolete route service tests and ensure proper functionality of the coordinator. - Refactored to simplify async setup tests and removed service registration assertions. - Enhanced with a new test for device association after migration, ensuring correct entity and device creation. - Deleted as its functionality is now covered in other tests. - Improved to validate station options formatting and ensure correct handling of various station formats.
- Updated test_migration.py to use AsyncMock for NSAPIWrapper and refactored legacy route migration tests. - Added new test_migration_entry.py to cover config entry migration scenarios. - Removed outdated tests from test_sensor.py related to service and trip sensors, adapting to new architecture. - Introduced test_station_parsing.py to validate station string parsing functionality. - Deleted test_sensor_past_trips.py as it was no longer relevant. - Updated test_subentry_flow.py to ensure time format consistency in route configurations.
…ion and reconfiguration steps. Update strings for improved clarity and consistency in route management.
- Implement tests for sensor logic in test_sensor.py, covering various scenarios including device association, availability, and native value handling. - Create new test files: test_sensor_edge_cases.py and test_sensor_new.py for additional edge case testing and new sensor functionalities. - Introduce utility tests in test_utils.py to validate time formatting, route validation, and safe data extraction methods. - Ensure coverage for scenarios with missing or invalid data in both sensors and utilities.
…ure and maintainability
…ed clarity and maintainability
|
@joostlek I have implemented the sub entries and made all the requested changes. Once we know if this is the final state I will update the docs again. |
| async def _async_migrate_legacy_routes( | ||
| hass: HomeAssistant, entry: NSConfigEntry | ||
| ) -> None: | ||
| """Migrate legacy routes from configuration data into subentries. | ||
|
|
||
| This handles routes stored in entry.data[CONF_ROUTES] from legacy YAML config. | ||
| One-time migration to avoid duplicate imports. | ||
| """ |
There was a problem hiding this comment.
Okay, so instead of importing the yaml into a config entry and only then migrating that config entry into subentries, we should migrate to subentries directly instead. Less moving objects, less room for error :)
There was a problem hiding this comment.
I'm actually already doing exactly what you suggested. The current _async_migrate_legacy_routes function directly converts YAML routes to subentries without any intermediate steps:
- Legacy YAML routes (stored in
entry.data[CONF_ROUTES]) - → Directly converted to ConfigSubentry objects
- → Added as subentries using
hass.config_entries.async_add_subentry(entry, subentry)
There's no intermediate step of creating separate config entries first. The flow is:
- Take YAML routes from
entry.data[CONF_ROUTES] - Create
ConfigSubentryobjects directly from that YAML data - Add them as subentries to the existing config entry
- Clean up the legacy
CONF_ROUTESfrom the main entry data
@joostlek
Could you clarify if I'm missing something or if there's a specific aspect of the migration that should be improved?
There was a problem hiding this comment.
Yes and no, you're running that code in async_setup_entry, which means that there already should be an entry to reach that code.
def setup_platform(
hass: HomeAssistant,
config: ConfigType,
add_entities: AddEntitiesCallback,
discovery_info: DiscoveryInfoType | None = None,
) -> None:This is the current entrypoint of the integration and we should use this (or the async counterpart) to start an import flow, and then the code in async_step_import in the config flow should make sure all the right subentries are created.
There was a problem hiding this comment.
@joostlek I have now added the logic to the async_setup as well. I have also run multiple tests locally and it imports the routes and config without issues
|
You should use |
@joostlek It is now fixed. Sorry had the wrong config missed the sensor and platform that was why I didn't see it. |
|
Will check! |
|
@joostlek did you have time to take a look? I wanted to update the docs PR but am first waiting on this one's go ahead before I finish that. |
|
Can you maybe send me a message on Discord? |
Proposed change
This PR enhances the Nederlandse Spoorwegen integration by implementing a full UI configuration flow and raising the integration quality to meet the Silver tier requirements. It adds robust reauthentication and reconfiguration support, improves error handling, and ensures a better user experience through clear validation and messaging. These changes make the integration easier to set up, maintain, and use within Home Assistant.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests:
Similar code found with 1 license type