Upgrade Swisscom integration#171816
Conversation
home-assistant
Bot
added
cla-signed
config-flow
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR modernizes the Swisscom Internet-Box integration by adding a config flow and refactoring device tracking to use an async DataUpdateCoordinator + entity-based device tracker.
Changes:
- Added config flow, constants, API client, and coordinator for async polling and authentication handling.
- Refactored
device_trackerplatform from legacyDeviceScannerto config-entry setup withScannerEntity+ coordinator. - Updated the integration manifest and added base UI strings for the config flow.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/swisscom/strings.json | Adds config flow strings for setup/abort/errors. |
| homeassistant/components/swisscom/manifest.json | Enables config flow and updates integration metadata. |
| homeassistant/components/swisscom/device_tracker.py | Migrates device tracker to config entries + coordinator + entities; adds YAML import deprecation issue. |
| homeassistant/components/swisscom/coordinator.py | Introduces polling coordinator that fetches and normalizes device data. |
| homeassistant/components/swisscom/const.py | Adds integration constants and defaults. |
| homeassistant/components/swisscom/config_flow.py | Implements UI and YAML import config flow with credential validation. |
| homeassistant/components/swisscom/api.py | Adds aiohttp-based client for the router web-services endpoint. |
| homeassistant/components/swisscom/init.py | Sets up config-entry lifecycle and forwards platforms. |
| }, | ||
| ) | ||
| if ( | ||
| result.get("type") is FlowResultType.ABORT |
| devices[device["Key"]] = { | ||
| "ip": device["IPAddress"], | ||
| "mac": device["PhysAddress"], | ||
| "host": device["Name"], | ||
| "active": device["Active"], |
| ir.async_create_issue( | ||
| hass, | ||
| HOMEASSISTANT_DOMAIN, | ||
| f"deprecated_yaml_{DOMAIN}", | ||
| breaks_in_ha_version="2027.5.0", | ||
| is_fixable=False, | ||
| is_persistent=False, | ||
| issue_domain=DOMAIN, | ||
| severity=ir.IssueSeverity.WARNING, | ||
| translation_key="deprecated_yaml", | ||
| translation_placeholders={ | ||
| "domain": DOMAIN, | ||
| "integration_title": "Swisscom Internet-Box", | ||
| }, | ||
| ) |
| for device in raw: | ||
| try: | ||
| devices[device["Key"]] = { | ||
| "ip": device["IPAddress"], | ||
| "mac": device["PhysAddress"], | ||
| "host": device["Name"], | ||
| "active": device["Active"], | ||
| } | ||
| except KeyError: | ||
| continue |
|
This superseeds this PR. |
| try: | ||
| async with self._session.post( | ||
| self._url, | ||
| json=payload, | ||
| headers={"Content-Type": CONTENT_TYPE, **headers}, | ||
| timeout=REQUEST_TIMEOUT, | ||
| ) as response: | ||
| if response.status == 401: | ||
| raise SwisscomAuthError("Unauthorized") | ||
| response.raise_for_status() | ||
| return await response.json(content_type=None) |
There was a problem hiding this comment.
So device specific logic should exist in a library hosted on PyPI, not in the integration
There was a problem hiding this comment.
Just so I understand, you're saying I should wrap the calls in a specific library and publish the package to PyPI?
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
joostlek
left a comment
joostlek
left a comment
There was a problem hiding this comment.
Awesome!
Okay so 3 other things:
- Because we migrate away from
DeviceScannerwe have more room to change stuff as we need to step away from the Scanner onto ScannerEntities. Usually we would ask to change to the library beforehand, but as that code wouldn't be used afterwards I am fine with moving to the library in this PR as well. - The config flow needs to be 100% unit tested
- I see that we add more YAML config to the device tracker platform. Let's not do that and just import the host if we have that. If we can't import it now because people have set a username or password, we can let users know in the issue we raise.
We merged the opnsense PR last week that had the same kind of migration, you can use that as example. Feel free to message me on Discord!
|
Thanks for the guidance @joostlek, really helpful, especially the pointer to the OPNsense PR. Since the new integration genuinely needs credentials and the legacy YAML can't carry them, I diverged slightly from the OPNsense pattern: instead of routing through async_step_import only to abort, async_setup_scanner raises a repair issue directly, telling the user to set up via UI and including their existing host as a placeholder for convenience. Is that fine this way? |
anatosun
force-pushed
the
upgrade/swisscom
branch
from
63baad2 to
6b7157e
Compare
|
Can you elaborate on why we now need credentials? With OPNSense the issue was that this was only supporting an OLD version, and the new one worked differently |
|
The /ws endpoint on recent Internet-Box firmware (I tested on IB4-00 running 14.20.40) no longer reliably responds to unauthenticated sah.Device.Information.get calls, it either returns an empty device list or a 401, depending on the firmware revision. The legacy scanner worked only because older firmware didn't enforce authentication on that endpoint. |
|
But it currently is still installed in 48 installations. How big is the chance that people are still running older firmware? |
Realistically very low, and I'd argue most of those 48 are already broken rather than quietly working passwordless. The unauthenticated Every working solution since then has required the router admin password: christophniederberger's patch in #89460, and schmidtfx's So requiring credentials isn't breaking working installs, those installs have been dead since 2023. It's the only way to make the integration work again on supported firmware. The only people who'd be on genuinely passwordless firmware are those who haven't taken a router update in 3+ years (e.g. an Internet-Box 2 that never got 13.20.x), which is a vanishing minority, and they get a clear repair issue pointing them to the UI setup rather than a silent failure. |
|
Okay understood. Let's go ahead then :) |
Check requirements
📦 python-swisscom-internet-box: 0.1.1
|
|
Should I do something more? |
|
Can you send open a documentation PR and send me a message on Discord? |
Breaking change
The Swisscom Internet-Box integration is no longer configured via YAML. Existing entries in
configuration.yamlare automatically imported into a config entry on the next restart, after which a repair issue will ask you to remove the YAML block fromconfiguration.yaml. YAML support will be removed in Home Assistant 2027.5.Authentication is now required. The legacy YAML platform queried
/wsunauthenticated, which no longer works reliably on recent Internet-Box firmware. During the YAML→config-entry import you will need to provide the administrator password (printed on the bottom of the box); the username defaults toadmin.Proposed change
Migrate the Swisscom Internet-Box integration from a legacy YAML-only
DeviceScannerto a UI config flow:config_flow.pywith auserand animportstep that asks for host, username (defaultadmin) and password, validates by performing the Sagemcomsah.Device.Information.createContexthandshake againsthttp://<host>/ws, and uses the box'sBaseMAC(fromDeviceInfo.get) as the config entry unique ID.api.py(SwisscomClient) that encapsulates the/wscalls and transparently re-authenticates once when the context ID expires.SwisscomDataUpdateCoordinatorpollingDevices.getevery 30 seconds and re-raising authentication failures asConfigEntryAuthFailed.device_tracker.pyrewritten to expose oneScannerEntityper LAN device known to the box, fed by the coordinator.PLATFORM_SCHEMA, butasync_setup_scannernow triggersSOURCE_IMPORTfor the config flow and raises the standarddeprecated_yamlrepair issue so existing users are migrated transparently.manifest.jsonupdated:config_flow: true,integration_type: hub, legacy quality scale marker removed.The change was validated against an IB4-00 box running firmware 14.20.40.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
Please allow me to finish my rejuvenation work before updating the documentation.
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: