deCONZ: Fix discovery script [3] #4192
Conversation
📝 WalkthroughWalkthroughAdds changelog entries through 8.5.0 and updates addon version. Reworks the deCONZ discovery script to read API keys from DB (primary/fallback), validate or create keys via the deCONZ API with retries, poll for a valid bridge ID (serial), and send discovery payloads to Home Assistant with updated logging and exit behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant S as Discovery Script
participant DB as deCONZ DB (primary / fallback)
participant A as deCONZ API
participant HA as Home Assistant
Note over S: Start discovery script
S->>DB: Read API key (primary)
alt not found
S->>DB: Read API key (fallback)
end
alt API key present
S->>A: GET /api/<key>/config (validate key)
alt valid
Note over S,A: Use existing key
else invalid
Note over S,A: Treat as missing -> create
loop up to 60 tries (1s)
S->>A: POST /api (create key)
A-->>S: response (apikey) or error
end
end
else no key
loop up to 60 tries (1s)
S->>A: POST /api (create key)
A-->>S: response (apikey) or error
end
end
loop up to ~25 tries (5s)
S->>A: GET /api/<key>/config (fetch bridgeid)
A-->>S: bridgeid or error
end
alt bridgeid valid (16-hex)
S->>HA: POST discovery payload {port, apikey, serial}
HA-->>S: success / failure
alt success
Note over S: Successfully sent discovery information
else failure
Note over S: Failed to send discovery message
end
else no valid bridgeid
Note over S: Exit with error (no serial)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
deconz/rootfs/etc/services.d/deconz/discovery (5)
23-31: Clarify log message when DB is missingThe script still attempts API-key creation and serial retrieval via API. “Proceeding without serial” is misleading here.
Apply:
- bashio::log.error "No database found in either path! Proceeding without serial." + bashio::log.error "No database found in either path! Will attempt API key creation and serial via API."
34-38: Guard sqlite3 usage to avoid noisy errorsIf sqlite3 is absent, skip DB read and fall back to API creation cleanly.
-if [ -n "$DATA_STORE" ]; then - api_key=$(sqlite3 "${DATA_STORE}" \ - 'SELECT apikey FROM auth WHERE devicetype="Home Assistant" ORDER BY createdate DESC LIMIT 1' 2>/dev/null) -fi +if [ -n "$DATA_STORE" ] && command -v sqlite3 >/dev/null 2>&1; then + api_key=$(sqlite3 "${DATA_STORE}" \ + 'SELECT apikey FROM auth WHERE devicetype="Home Assistant" ORDER BY createdate DESC LIMIT 1' 2>/dev/null) || true +else + bashio::log.debug "Skipping database read (no DATA_STORE or sqlite3 not available)." +fi
40-51: Add curl timeouts when validating existing API keyPrevents hangs if local API is slow/unresponsive.
- http_code=$(curl -s -o /dev/null -w "%{http_code}" \ + http_code=$(curl --silent --connect-timeout 2 --max-time 5 -o /dev/null -w "%{http_code}" \ "http://127.0.0.1:40850/api/${api_key}/config")
52-71: Harden API key creation loop (timeouts) and surface error contextAvoid indefinite waits and improve diagnostics.
- if result="$(curl --silent --show-error --request POST \ + if result="$(curl --silent --show-error --connect-timeout 2 --max-time 5 --request POST \ -d '{"devicetype": "Home Assistant"}' \ "http://127.0.0.1:40850/api")"; then api_key="$(bashio::jq "${result}" '.[0].success.username')" if bashio::var.has_value "${api_key}"; then bashio::log.info "API key successfully created after ${i} seconds." break fi + # Log error description if provided by deCONZ + err_desc="$(bashio::jq "${result}" '.[0].error.description')" + if bashio::var.has_value "${err_desc}"; then + bashio::log.debug "API key create attempt ${i} response: ${err_desc}" + fi fi
73-89: Try fetching bridge ID immediately; add curl timeoutsReduces time-to-success and avoids hangs.
-while [[ "${serial}" = "0000000000000000" && "${retries}" -gt 0 ]]; do - sleep 5 - if result="$(curl --silent --show-error \ +while [[ "${serial}" = "0000000000000000" && "${retries}" -gt 0 ]]; do + if result="$(curl --silent --show-error --connect-timeout 2 --max-time 5 \ --request GET "http://127.0.0.1:40850/api/${api_key}/config")"; then candidate="$(bashio::jq "${result}" '.bridgeid')" if [[ "$candidate" =~ ^[0-9A-Fa-f]{16}$ ]]; then serial="$candidate" else serial="0000000000000000" fi fi ((retries--)) + if [[ "${serial}" = "0000000000000000" && "${retries}" -gt 0 ]]; then + sleep 5 + fi done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
deconz/CHANGELOG.md(1 hunks)deconz/rootfs/etc/services.d/deconz/discovery(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
*/**(html|markdown|md)
⚙️ CodeRabbit configuration file
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
*/**(html|markdown|md): - Use bold to mark UI strings.
If "" are used to mark UI strings, replace them by bold.
Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Use sentence-style capitalization also in headings.
do not comment on HTML used for icons
Avoid flagging inline HTML for embedding videos in future reviews for this repository.
Files:
deconz/CHANGELOG.md
🔇 Additional comments (3)
deconz/CHANGELOG.md (1)
3-6: Changelog entry looks goodVersion section and note are clear and consistent with prior entries.
deconz/rootfs/etc/services.d/deconz/discovery (2)
12-12: Good: explicit start logHelpful for tracing startup.
103-108: Success/error logs on discovery send look goodClear operator feedback on outcome.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
deconz/rootfs/etc/services.d/deconz/discovery (1)
73-88: Move sleep to end of loop for faster first attempt.The
sleep 5at line 77 executes before the first attempt, causing an unnecessary 5-second delay. Moving it to the end of the loop will make the first attempt immediate while still maintaining the 5-second interval between retries.Apply this diff:
-retries=25 -serial="0000000000000000" -while [[ "${serial}" = "0000000000000000" && "${retries}" -gt 0 ]]; do - sleep 5 - if result="$(curl --silent --show-error \ - --request GET "http://127.0.0.1:40850/api/${api_key}/config")"; then - candidate="$(bashio::jq "${result}" '.bridgeid')" - if [[ "$candidate" =~ ^[0-9A-Fa-f]{16}$ ]]; then - serial="$candidate" - else - serial="0000000000000000" - fi - fi +retries=25 +serial="0000000000000000" +while [[ "${serial}" = "0000000000000000" && "${retries}" -gt 0 ]]; do + if result="$(curl --silent --show-error \ + --request GET "http://127.0.0.1:40850/api/${api_key}/config")"; then + candidate="$(bashio::jq "${result}" '.bridgeid')" + if [[ "$candidate" =~ ^[0-9A-Fa-f]{16}$ ]]; then + serial="$candidate" + else + serial="0000000000000000" + fi + fi ((retries--)) + [[ "${serial}" = "0000000000000000" && "${retries}" -gt 0 ]] && sleep 5 done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
deconz/config.yaml(1 hunks)deconz/rootfs/etc/services.d/deconz/discovery(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- deconz/config.yaml
🔇 Additional comments (6)
deconz/rootfs/etc/services.d/deconz/discovery (6)
12-13: LGTM!Good addition for observability. The start log helps track when the discovery process begins.
23-32: LGTM!The fallback logic correctly addresses the database path issue mentioned in the PR. The graceful degradation (proceeding without serial if no database is found) prevents the service from failing.
34-50: LGTM!The API key validation logic is well-implemented. Reading from the database when available and validating via HTTP before use is good defensive programming.
52-71: LGTM!The API key creation loop is well-structured with appropriate retry logic. The 60-second window gives users time to unlock the gateway, and the non-fatal exit prevents service flapping.
90-93: LGTM! Past review comment addressed.The non-fatal exit (status 0) prevents s6 service restarts when the bridge ID cannot be retrieved, which was correctly identified in the previous review.
105-107: LGTM!The updated log messages provide clearer feedback on the discovery process outcome, improving observability.
|
I encountered a problem: If the discovery script works, all users with the integration should switch to the "Home Assistant" API key. This will fix the internal communication and this issue. I'm not entirely sure whether the "pydeconz" API key needs to be deleted first, but I've already thought of a workaround for that. The DOCS.md could be extended with this. My deCONZ integration can no longer be activatedIf this issue occurs, first remove the integration and check the API keys used in the Phoscon App. Go to Help -> API -> Information -> Configuration and scroll to "Whitelist". Here, only the "Home Assistant" API key is required for Home Assistant, not "pydeconz". To do this, install "Advanced SSH & Web Terminal" in Home Assistant, for example, and send the following expression to the API to remove the key. After a subsequent system restart, you can reinstall the deCONZ integration.
curl --request DELETE "http://127.0.0.1:<(API) Port>/api/<apikey>/config/whitelist/<apikey2>" |
mattreim
changed the title
1 participant
Unfortunately the discovery script never worked because the path was incorrect. I also improved it a bit and added more log data.
Summary by CodeRabbit
Bug Fixes
Documentation