-
-
Notifications
You must be signed in to change notification settings - Fork 36.3k
Correctly pass MopParserConfig for Roborock #157891
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correctly pass MopParserConfig for Roborock #157891
Conversation
|
Hey there @allenporter, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
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 fixes a bug introduced during a refactor where the MapParserConfig was not being properly passed to the create_device_manager function in the Roborock integration. The fix restores the map parser configuration that was lost in the refactor, ensuring that map rendering options (like drawables, background visibility, and scale) are correctly applied.
Key changes:
- Added
MapParserConfigimport and related constants to properly configure map parsing - Restored the
map_parser_configparameter in thecreate_device_managercall with proper construction of drawables, background settings, and scale
| map_parser_config=MapParserConfig( | ||
| drawables=[ | ||
| drawable | ||
| for drawable, default_value in DEFAULT_DRAWABLES.items() | ||
| if entry.options.get(DRAWABLES, {}).get(drawable, default_value) | ||
| ], | ||
| show_background=entry.options.get(CONF_SHOW_BACKGROUND, False), | ||
| map_scale=MAP_SCALE, | ||
| ), |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description indicates that "Tests have been added to verify that the new code works," but there are no test changes visible in this PR to verify that the map_parser_config parameter is being correctly passed to create_device_manager.
Consider adding a test that verifies:
- The
map_parser_configparameter is passed tocreate_device_manager - The
MapParserConfigis constructed with the correct values fromentry.options - The drawables list comprehension correctly filters based on the options and defaults
Example test:
async def test_map_parser_config_passed(
hass: HomeAssistant,
mock_roborock_entry: MockConfigEntry,
fake_create_device_manager: Mock,
) -> None:
"""Test that map_parser_config is correctly passed to create_device_manager."""
await hass.config_entries.async_setup(mock_roborock_entry.entry_id)
await hass.async_block_till_done()
# Verify create_device_manager was called with map_parser_config
assert fake_create_device_manager.call_args.kwargs["map_parser_config"] is not None
map_config = fake_create_device_manager.call_args.kwargs["map_parser_config"]
assert map_config.map_scale == MAP_SCALE
Breaking change
Proposed change
This fixes a bug from the refactor where the mop parser config is not properly passed in. This is a copy of how we previously made the data for config.
Should be included in patch version.
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.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: