Skip to content

Clean up duplicate work in publish_device_info - #324

Merged
1technophile merged 2 commits into
theengs:developmentfrom
jcsanyi:discovery-cleanup
Aug 15, 2026
Merged

Clean up duplicate work in publish_device_info#324
1technophile merged 2 commits into
theengs:developmentfrom
jcsanyi:discovery-cleanup

Conversation

@jcsanyi

@jcsanyi jcsanyi commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Description:

Two independent cleanups to DiscoveryGateway.publish_device_info, neither of which changes what gets published. They came out of the review on #323 and are split out here so that PR can be just the feature.

Only parse decoder properties once. getProperties was called and parsed twice for the same model - once into pub_device["properties"], once into a local data - and the loop then iterated one copy while reading each property's metadata out of the other. The loop now iterates pub_device["properties"] directly. With a single copy left, the if k in pub_device["properties"] guard around the metadata lookup is trivially true, so it goes as well and the block dedents. It was never doing anything; both copies came from the same getProperties output, so the key was always present.

Publish device_tracker discovery once per device. publish_device_tracker was called from inside the per-property loop but doesn't depend on the loop variable, so it republished the same retained config once per decoder property - twice for HOLYIOT, BM2 and BM6, four times for MB/SW. Moving the call above the loop publishes it once. It still runs ahead of the per-property configs, so the order on the wire is unchanged.

That second one keeps the intent of #232, which moved the call to the top of the loop body because tracker-only devices declare a single device property that hits the continue below - at the bottom of the loop it never fired for them. Above the loop it can't be skipped at all. I checked that case explicitly: TILE (one device property) publishes its tracker config exactly once before and after.

Verification

I stubbed publish on a bare DiscoveryGateway and captured every call - retained config payloads keyed by topic, plus a raw call count per topic - across a set of models picked to hit each branch of the loop: CGPR1 (binary_sensor via motion, mac skip, and the entity_type carry-over between iterations), CGDK2 and TPMS (several measurement properties), IBEACON (nothing skipped), APPLEWATCH (the unlocked val_tpl special case), LYWSD03MMC/MJWSD05MMC_PVVX_ENCR (cipher/ctr/mic skips), HOLYIOT and MB/SW (tracked, 2 and 4 properties), and TILE (tracker-only).

Retained configs are byte-identical to development for every one of them. The only thing that changes is the tracker publish count: HOLYIOT 2 → 1, MB/SW 4 → 1, TILE 1 → 1. I ran the first commit on its own as well - identical configs and identical counts, as expected, since the republishing is untouched at that point.

Checklist:

  • I have created the pull request against the latest development branch
  • I have added only one feature/fix per PR and the code change compiles without warnings
  • I accept the Developer Certificate of Origin (DCO).

getProperties was called and parsed twice for the same model, then the
loop iterated one copy while reading each property's metadata out of
the other. Iterating pub_device["properties"] directly drops the second
parse.

With one copy left, the `if k in pub_device["properties"]` guard around
the metadata lookup is trivially true, so it goes as well and the block
dedents. It was never doing anything: both copies came from the same
getProperties output, so the key was always present.

No change to what gets published: same topics, same payloads.
publish_device_tracker was called from inside the per-property loop but
doesn't depend on the loop variable, so it re-published the same
retained config once per decoder property: twice for HOLYIOT, BM2 and
BM6, four times for Miband.

Moving the call above the loop publishes it once. It stays ahead of the
per-property configs, so the order on the wire is unchanged.

This keeps the intent of theengs#232, which moved the call to the top of the
loop body because tracker-only devices declare a single "device"
property that hits the `continue` below. Above the loop it can't be
skipped at all.
@DigiH

DigiH commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Only parse decoder properties once. getProperties was called and parsed twice for the same model - once into pub_device["properties"], once into a local data

Just fo9r verification can yo9u turn on Advanced and Advertising Data with

-padv PUBLISH_ADVDATA, --publish_advdata PUBLISH_ADVDATA
                        Publish advertising and advanced data (1) or not (0)
                        (default: 0)

and see if they are both still identical, or if data might only contain the device decoder defined properties, while pub_device["properties"]might also contain all the advanced and advertising data, only required for conditionals, but not for iterating through the discovery loop.

@jcsanyi

jcsanyi commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Good idea - I tested with publish_advdata both on and off, and the discovery configs are identical either way, before and after this change.

They can't actually diverge: pub_device["properties"] is unconditionally overwritten from the same getProperties(model_id) call a few lines above where the old code parsed data, and getProperties doesn't depend on the gateway config. The advertising data lives as top-level keys on pub_device itself, never inside pub_device["properties"].

I also verified this live on my production gateway with PUBLISH_ADVDATA=true against a SwitchBot Outdoor Meter (W340001X): with advdata on, the state payload gains manufacturerdata/servicedata/servicedatauuid/acts as expected, but the retained discovery configs are byte-identical between 1.7.5 and this branch, and identical with the flag off.

@1technophile

Copy link
Copy Markdown
Member

Thanks

@1technophile
1technophile merged commit 271f736 into theengs:development Aug 15, 2026
@jcsanyi
jcsanyi deleted the discovery-cleanup branch August 15, 2026 14:49
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