Skip to content

feat(config): propose the declared indoor monitor for Call Home - #173

Merged
lollox80 merged 4 commits into
ha-vimar:mainfrom
andreacasarin:feat/internal-panel-from-nicknames
Oct 14, 2026
Merged

lollox80 merged 4 commits into
ha-vimar:mainfrom
andreacasarin:feat/internal-panel-from-nicknames

Conversation

@andreacasarin

@andreacasarin andreacasarin commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Call Home's default indoor-panel address is not correct on every plant. On the Tab 7S Up 40517 in #172, GET_NICKS declared PICG 60001, distinct from SGA 61000; using that PICG made Call Home ring the physical monitor. On the 40507 development plant, PICG is also the SGA/controller, and Call Home uses another address. A PICG declaration alone therefore cannot identify the monitor on every plant.

Add Configure → Find indoor monitor. It proposes a single validated PICG extension only when it differs from the effective SGA (sga_target or SGA_TARGET). When they coincide, a neutral conditional explanation in the form description keeps this informational rather than showing a red error; the current Call Home destination, including its existing default, is preserved. Saving requires explicit confirmation and changes only the indoor-panel option.

Cached nicknames containing a PICG are reused. A cache without a PICG triggers GET_NICKS to only the configured address. Missing nickname replies and unavailable lookups (disconnected/missing hub or an active scan) have separate messages from a reply without a unique valid PICG. The flow uses rest_client.ROLE_PICG and retains strict target and ambiguity checks. A known monitor address can still be entered manually after any lookup error.

The step does not infer addresses from numbering, scan ranges or place calls.

Tested on

The PICG-to-monitor mapping and successful Call Home were verified on an Elvox Tab 7S Up 40517, SW 2.1.0203 / FW 1.06, cloud TLS, Home Assistant Container. The new options UI has not yet been field-tested on that plant. The 40507 counterexample and cloud 40515 no-reply behavior were supplied in review; they are covered by synthetic regression cases, not new hardware measurements by this contributor.

Tests cover empty/explicit SGA and indoor target values, cached/queried nicknames, timeout and accepted-without-reply results, unavailable scans, missing/invalid/ambiguous declarations, explicit confirmation, and preservation of unrelated settings. The combined branch with #171, #174 and #175 is also tested.

Validation: standalone default suite 2,199 passed; combined suite 2,218 passed; each has 1 skipped and 204 deselected, line and branch coverage 95.89%. Whole-repository ruff check . passes.

Docs

Docs: docs/CONFIGURATION.md and docs/CONFIGURATION.it.md; English and Italian options-flow translations.

Changelog

Changelog: Enhancements - Propose a declared PICG as the Call Home destination when it differs from the SGA, preserving the current destination otherwise and distinguishing unavailable lookups from missing replies.

@Apeiv

Apeiv commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Nice addition. Tests pass here (Python 3.14, also with #171, #174 and #175 merged on top).

  1. On the 40507 dev plant the PICG is not the monitor. Its rubrica.db declares PICG 55001, which is also the SGA ("Casa CG", docs/RUBRICA.md:118-120), while Call Home defaults to 55002 there (const.py:106-111). So this step would prefill 55001 over a working 55002 (options_flow.py:377). Maybe skip the proposal, or keep the current value as default, when the declared PICG equals the effective SGA (sga_target or SGA_TARGET, since sga_target can be empty)? On the 40517 and on our 40515 (PICG 60001, SGA 61000, Call Home 60001) the mapping holds.

  2. On a cloud 40515 GET_NICKS to an existing address can get no SIP answer at all (issue Find the SGA/PICG without the phonebook: a scan built on the three-outcome rule #14), so the step waits about 20 s and then shows internal_panel_not_found, which blames connectivity and the PICG. Same message when async_find_picg returns ok: False because another scan is running or the hub is gone. A separate "no reply" message would point people the right way.

  3. options_flow.py:363: a cached nickname list without a PICG entry never triggers a fresh request, so the step reports not found without asking. Checking for a PICG in the cache instead of not nicks would cover it.

  4. Nit: async_find_picg already returns a "picg" key that gets recomputed here from nicknames. Keeping your stricter check (reject ambiguous, validate) is good, but rest_client.ROLE_PICG could replace the "PICG" literal at :367.

Happy to run the step on our 40515 and report what it shows.

@andreacasarin

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review. I’ve addressed all four points in commit f4fc8bd:

  1. When the declared PICG equals the effective SGA (sga_target or SGA_TARGET), the form keeps the current Call Home destination, including its default, and explains why it cannot propose the PICG as the monitor.
  2. Separate messages now distinguish a missing nickname reply, an unavailable lookup, and a reply without a unique valid PICG.
  3. Cached nicknames without a PICG entry now trigger a fresh GET_NICKS request to the configured PICG.
  4. The role check uses rest_client.ROLE_PICG, retaining target validation and ambiguity rejection.

The updated PR passes 2,199 default tests. With #171, #174 and #175 applied, 2,218 tests pass. Both runs reach 95.89% line and branch coverage, and ruff check . passes.

These changes have automated coverage; the updated options flow has not yet been field-tested on my 40517. Could you try the updated branch on your 40515 and report what the step shows, particularly whether GET_NICKS returns a declaration or the new no-reply message?

Thanks again for offering to test it.

@Apeiv

Apeiv commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Tried f4fc8bd on our 40515 (cloud TLS + SRTP, PICG 60001, SGA 61000, Call Home 60001), with only this PR's changes on top of our current build.

Configure → Find indoor monitor sent GET_NICKS to 60001 only. The usual 407 came first, then the retry got a 200 and the reply arrived about 0.3 s after the first send:

GET_NICKS_REPLY: 3 entries, declared PICG=60001
[{"EXT":"60001","ROLE":"PICG",...},{"EXT":"<our id>","ROLE":"PIM","NAME":"Home Assistant"},{"EXT":"<another phone>","ROLE":"PIM",...}]

That makes 60001 the proposal, the address Call Home already uses here. I drove the flow through the API and kept 60001, so I didn't look at the form itself. Nothing rang, nothing else changed. I didn't get to test the no-reply path, since the panel always answered.

On this plant GET_NICKS to an existing address does answer, so the "no SIP answer on a cloud 40515" case from the async_find_picg docstring (issue #14) didn't happen here. Maybe a firmware or plant difference. Your no-reply message is still worth keeping for the plants where it happens.

Works for us, thanks for the quick turnaround!

@lollox80 lollox80 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, and thanks @Apeiv for the 40515 run. Read through f4fc8bd: the logic matches what async_find_picg returns on main (queued/exists without a reply → no-reply message, ok: False → unavailable, a reply without a unique valid PICG → not found), and the PICG = SGA guard covers the 40507 case. A few small points, none blocking:

  1. options_flow.py (async_step_internal_panel, the internal_panel_same_as_sga branch): this is informational, not a failure, but as errors["base"] HA shows it in red as if the lookup had failed. Could it go into the description instead (e.g. a {note} placeholder), keeping errors for real failures?
  2. Same function, the late-reply path: when a GET_NICKS reply arrives after reply_wait and carries no PICG, async_find_picg returns reply_after: None and no replied probe, so the user sees "no reply" although a reply did arrive. Edge case; fine to leave, maybe worth a one-line comment.
  3. sip_timeout=20, reply_wait=10: the form can sit for ~30 s with only a spinner. A shorter reply_wait (Apeiv's reply came in ~0.3 s) would cut the worst case; your call.
  4. Nit, docs (CONFIGURATION.md / .it.md): the last two sentences ("Cached nicknames without a PICG…", "No reply and an unavailable lookup…") describe internals more than what the user does; they could be dropped.

@lollox80

Copy link
Copy Markdown
Collaborator

Field test of f4fc8bd on my 40507 (2-wire, local UDP), the PICG = SGA case:

  • Configure → Rileva il posto interno read the declared PICG 55001 and recognised that it is also the SGA.
  • It did not propose 55001. The error box explains that the reply can't identify the PICG as an indoor monitor and that the current Call Home destination is kept.
  • The field stayed on 55002, the working Call Home destination. Nothing rang and nothing else in the options changed. I closed the form without saving.

So the branch added after @Apeiv's review behaves as intended on real hardware. With Apeiv's run on his 40515 (PICG ≠ SGA, proposal = 60001), both sides are now covered. What's left is your run on the 40517 through the form.

Optional nit on the wording: the intro text is the same in both cases ("Viene proposto un PICG diverso dall'SGA; se coincidono si mantiene la destinazione attuale…"). When nothing is proposed, as here, the first half reads oddly. Either two texts (one per case), or a neutral sentence such as "Se il PICG dichiarato è diverso dall'SGA viene proposto come posto interno; altrimenti resta la destinazione attuale." (and the same in English). Fine to leave for a follow-up.

@andreacasarin

andreacasarin commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor Author

Tested on a 40517, Home Assistant 2026.8.3 Container, with all four PRs installed together.

Find indoor monitor correctly proposes 60001. After confirmation, the destination remains 60001 and Call Home rings the physical indoor monitor.

Screenshot 2026-10-13 at 14 23 37 Screenshot 2026-10-13 at 14 24 08 Screenshot 2026-10-13 at 14 25 11

@lollox80
lollox80 added this pull request to the merge queue Oct 14, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 14, 2026
Resolve the conflicts with ha-vimar#174 by keeping both sides: the flow's
_internal_panel_suggestion and _phonebook_source attributes, and the
internal panel tests next to the import summary test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XMtKjPuZNEaX23BpL9AsaA
@lollox80
lollox80 added this pull request to the merge queue Oct 14, 2026
Merged via the queue into ha-vimar:main with commit 2191619 Oct 14, 2026
13 checks passed
lollox80 added a commit that referenced this pull request Oct 14, 2026
Resolve the conflict in tests/test_config_flow_steps.py by keeping both
sides: the outgoing video option tests and the internal panel and import
summary tests from #173 and #174.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XMtKjPuZNEaX23BpL9AsaA
@lollox80 lollox80 mentioned this pull request Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants