deCONZ: Update ingress, docs and a few corrections [2] #4161
Conversation
📝 WalkthroughWalkthroughVersion bumped to 8.4.0. Documentation, translations, and UI text updated to prefer Phoscon App terminology and lowercase "dresden elektronik". Ingress page rebuilt with metadata, CSS variables (light/dark), responsive layout, and a lightweight ripple interaction. Minor branding capitalization fixes in OTA scripts. No runtime API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Ingress
participant Phoscon
participant deCONZ
rect rgb(248,249,255)
User->>Ingress: Open ingress URL
Ingress-->>User: Render card UI (theme vars + buttons)
end
rect rgb(240,255,240)
User->>Ingress: Click "Phoscon"
Note right of Ingress #dff7e2: ripple animation then navigate
Ingress-->>Phoscon: Open Phoscon App URL (target="_blank")
end
rect rgb(255,245,235)
User->>Ingress: Click "deCONZ"
Note right of Ingress #ffe8d6: ripple animation then navigate (VNC/GUI)
Ingress-->>deCONZ: Open deCONZ GUI/VNC URL (target)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
deconz/config.yaml (1)
37-37: Align docs/translations with singulardeviceschema
- deconz/DOCS.md documents "Option:
devices" (deconz/DOCS.md:151) while the add-on option in the schema isdevice(deconz/config.yaml:28,37). Update DOCS.md to usedevice.- deconz/translations/en.yaml uses the key
device(deconz/translations/en.yaml:3) but the display name is "Devices" (same in de.yaml). Make the display label singular ("Device"/"Gerät") or add a short clarifying note that top-level manifestdevices(deconz/config.yaml:16) is the host-permissions list and the user-facing option isdevice.deconz/rootfs/etc/services.d/otau-ledvance/run (1)
33-37: Bug: DL_DONE is not incremented
((DL_DONE+1))is a no-op; rate limiting will never trigger, risking API throttling.Apply:
- ((DL_DONE+1)) + ((DL_DONE+=1))
🧹 Nitpick comments (12)
deconz/CHANGELOG.md (1)
3-10: Polish wording for style consistencyPrefer “for example” over “e.g.” and expand language names for clarity.
Apply:
-## 8.3.0 - -- Ingress entry page revised (e.g. light/dark mode) -- Add de translation and en translation expanded +## 8.3.0 + +- Ingress entry page revised (for example, light and dark mode) +- Add German translation and expand English translationdeconz/DOCS.md (6)
31-31: Use bold UI path, not code styling.Align with guide and earlier steps.
-`Settings -> System -> Hardware -> All hardware` page. It is recommended +**Settings** -> **System** -> **Hardware** -> **All hardware** page. It is recommended
36-39: Remove HTML line break and tighten example lead-in.Avoid inline HTML and keep instructions direct.
- the device name in quotes: e.g. something like <br> + the device name; for example:
43-45: Format UI button names with bold and remove curly quotes.Follow UI string formatting rule.
-After installing and starting this add-on, you can use the “deCONZ” button to access the -deCONZ GUI via VNC or the “Phoscon” button to configure further settings in the Phoscon App. +After installing and starting this add-on, select **deCONZ** to access the deCONZ GUI via VNC, +or select **Phoscon** to configure settings in the Phoscon App.
92-95: Make firmware upgrade instructions concise.Remove fluff and front the goal.
-This add-on allows you to upgrade your firmware straight from the Phoscon App with ease. - -Go to `Settings -> Gateway` and click the upgrade button. +Upgrade firmware in the Phoscon App. + +In the Phoscon App, go to **Settings** -> **Gateway**, then select the upgrade button.
109-110: Fix article and simplify the warning.Grammar and tone.
-**WARNING:** Do not use the following settings to set up a integration manually -from within Home Assistant! +**WARNING:** Do not use these settings to set up an integration in Home Assistant.
156-156: Unify the UI path.Earlier you used “Settings -> System -> Hardware -> All hardware”. Keep it consistent here.
-`Settings -> Hardware -> All hardware` page. It is recommended +**Settings** -> **System** -> **Hardware** -> **All hardware** page. It is recommendeddeconz/rootfs/usr/share/www/ingress.html (2)
3-4: Add basic metadata for accessibility and responsive layout.Add lang, charset, and viewport.
-<html> - <head> +<html lang="en"> + <head> + <meta charset="utf-8"> + <meta name="viewport" content="width=device-width, initial-scale=1"> + <title>deCONZ | dresden elektronik</title>
208-211: Harden external links.Include noopener with noreferrer to prevent reverse tabnabbing.
- <a href="https://phoscon.de/en/conbee/software#phoscon-app" target="_blank" rel="noreferrer">Phoscon App</a> + <a href="https://phoscon.de/en/conbee/software#phoscon-app" target="_blank" rel="noopener noreferrer">Phoscon App</a> @@ - <a href="https://phoscon.de/en/conbee/software#deconz" target="_blank" rel="noreferrer">deCONZ GUI</a> + <a href="https://phoscon.de/en/conbee/software#deconz" target="_blank" rel="noopener noreferrer">deCONZ GUI</a>deconz/rootfs/etc/services.d/deconz/discovery (3)
53-60: Use timeouts when validating the existing API key.Avoids blocking on network issues.
- http_code=$(curl -s -o /dev/null -w "%{http_code}" \ + http_code=$(curl -sS --connect-timeout 2 --max-time 4 -o /dev/null -w "%{http_code}" \ "http://${TARGET_IP}:${API_PORT}/api/${api_key}/config")
67-75: Set content type and timeouts when creating the API key.Improves robustness with the deCONZ API.
- if result="$(curl --silent --show-error --request POST \ - -d '{"devicetype": "Home Assistant"}' \ + if result="$(curl --silent --show-error --connect-timeout 2 --max-time 4 \ + --request POST -H 'Content-Type: application/json' \ + -d '{"devicetype": "Home Assistant"}' \ "http://${TARGET_IP}:${API_PORT}/api")"; then
85-93: Add timeouts when fetching bridge ID.Same reason: avoid stalls.
- if result="$(curl --silent --show-error \ + if result="$(curl --silent --show-error --connect-timeout 2 --max-time 4 \ --request GET "http://${TARGET_IP}:${API_PORT}/api/${api_key}/config")"; then
📜 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 (12)
README.md(1 hunks)deconz/CHANGELOG.md(1 hunks)deconz/DOCS.md(6 hunks)deconz/Dockerfile(1 hunks)deconz/README.md(1 hunks)deconz/config.yaml(1 hunks)deconz/rootfs/etc/services.d/deconz/discovery(1 hunks)deconz/rootfs/etc/services.d/otau-ikea/run(1 hunks)deconz/rootfs/etc/services.d/otau-ledvance/run(1 hunks)deconz/rootfs/usr/share/www/ingress.html(1 hunks)deconz/translations/de.yaml(1 hunks)deconz/translations/en.yaml(2 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.mddeconz/README.mddeconz/Dockerfiledeconz/config.yamldeconz/DOCS.md
🪛 GitHub Check: YAMLLint
deconz/translations/de.yaml
[failure] 18-18:
line too long
[failure] 17-17:
line too long
deconz/translations/en.yaml
[failure] 18-18:
line too long
[failure] 17-17:
line too long
🪛 GitHub Actions: Lint
deconz/translations/en.yaml
[error] 17-17: yamllint: line-length violation on line 17:81: line too long (84 > 80 characters) in ./deconz/translations/en.yaml.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build aarch64 deconz add-on
- GitHub Check: Build armhf deconz add-on
🔇 Additional comments (12)
deconz/README.md (1)
5-5: Branding casing aligned with manufacturerLowercasing to “dresden elektronik” matches the official style and is consistent across the PR.
README.md (1)
22-22: Consistent brandingCasing updated to “dresden elektronik” is correct and consistent with other files.
deconz/rootfs/etc/services.d/otau-ikea/run (1)
15-15: Correct brand capitalization in log“IKEA” capitalization fix is correct.
deconz/Dockerfile (1)
38-38: Reduce image size: apt cache cleanupAdding
apt-get cleanin the same RUN layer is correct and reduces image size. LGTM.deconz/config.yaml (2)
2-2: Version bump8.3.0 matches the changelog entry.
6-6: Description brandingCasing change to “dresden elektronik” is correct.
deconz/rootfs/etc/services.d/otau-ledvance/run (1)
5-5: Brand capitalization fixes“OSRAM LEDVANCE” capitalization in header and logs is correct.
Also applies to: 12-12, 16-16
deconz/translations/en.yaml (1)
4-4: Label pluralization looks good."Devices" matches the docs intent and branding updates.
deconz/DOCS.md (3)
182-184: Tighten step wording.[ suggest_nitpick ]
- **Settings** -> **Gateway** -> **Backup options** button, next create + **Settings** -> **Gateway** -> **Backup options**, then create
189-189: Clarify restore instruction.[ suggest_nitpick ]
-6. Restore the backup you created at the first step at the same location in the Phoscon App as before. +6. Restore the backup at the same location in the Phoscon App as in step 1.
151-151: Option name mismatch — config.yaml contains bothdevices:anddevice:config.yaml shows a top-level
devices:(line 16) and indenteddevice:entries (lines 28, 37). Confirm which key is the intended add-on option and make DOCS.md match the schema.
- If the schema key is
device, change the header in DOCS.md to:Option:
device(required)- If the schema key is
devices, update examples/translations to usedevices.config.yaml — lines: 16, 28, 37.
deconz/rootfs/etc/services.d/deconz/discovery (1)
101-106: Confirm host value expected by HA discovery consumersrg returned no matches for bashio::discovery; cannot verify consumers in repo. Confirm whether discovery expects the documented alias "core-deconz" instead of hostname—if so, change:
- host "$(hostname)" \ + host "core-deconz" \
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
deconz/rootfs/etc/services.d/deconz/discovery (2)
23-37: Resolved earlier concern: reachability loop now uses curl timeouts and exits cleanly.This addresses the prior “add timeouts and fail path” feedback.
77-78: Good: API key no longer logged.This addresses the credential exposure concern from earlier review.
🧹 Nitpick comments (3)
deconz/translations/de.yaml (1)
7-15: Consistency: capitalize protocol acronyms (APS/ZCL/ZDP/OTAU) and “Info”German/English mix is fine, but acronyms are typically uppercase; this matches deCONZ terms and improves UI polish. Align with en.yaml if it already defines the style.
dbg_aps: - name: Debug aps + name: Debug APS dbg_info: - name: Debug info + name: Debug Info dbg_otau: - name: Debug otau + name: Debug OTAU dbg_zcl: - name: Debug zcl + name: Debug ZCL dbg_zdp: - name: Debug zdp + name: Debug ZDPdeconz/rootfs/etc/services.d/deconz/discovery (2)
20-22: Nice: centralized API host/port.Good step toward configurability. Minor: consider marking them readonly.
-TARGET_IP="127.0.0.1" -API_PORT="40850" +declare -r TARGET_IP="127.0.0.1" +declare -r API_PORT="40850"
50-55: Guard sqlite3 usage to avoid hard failure on missing binary.Add a quick pre-flight check and skip the DB read if sqlite3 isn’t present.
-# Read API key from database -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 +# Read API key from database +if [ -n "$DATA_STORE" ]; then + if 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) + else + bashio::log.warning "sqlite3 not available; skipping DB lookup for API key." + fi +fi
📜 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 (3)
deconz/rootfs/etc/services.d/deconz/discovery(1 hunks)deconz/translations/de.yaml(1 hunks)deconz/translations/en.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- deconz/translations/en.yaml
🧰 Additional context used
🪛 GitHub Check: YAMLLint
deconz/translations/de.yaml
[failure] 20-20:
line too long
🪛 GitHub Actions: Lint
deconz/translations/de.yaml
[error] 20-20: yamllint failed on ./deconz/translations/de.yaml: line too long (85 > 80 characters) (line-length)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build armhf deconz add-on
- GitHub Check: Build aarch64 deconz add-on
🔇 Additional comments (3)
deconz/rootfs/etc/services.d/deconz/discovery (3)
39-49: DB fallback path looks good.The alternate dresden-elektronik location and clear logs will help with migrations.
106-111: Confirm discovery schema expects numeric port (using caret).Using “^” to emit a JSON number is typical for bashio, but please confirm Home Assistant’s deCONZ discovery schema expects an integer here.
Would you like me to open a follow-up to check the integration’s discovery schema and adjust if it expects a string?
115-118: Clear success/failure logs on discovery send.Helpful for users and support triage.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
deconz/rootfs/etc/services.d/deconz/discovery (1)
23-33: Fix success-at-attempt-30 bug in readiness loop.
If the API becomes reachable exactly on the 30th try, i remains 30 and the script exits as “not reachable.”Use a ready flag:
# Wait until deCONZ API is reachable bashio::log.info "Waiting for deCONZ API to be available..." +ready=0 for i in $(seq 1 30); do if curl -fsS --connect-timeout 1 --max-time 2 -o /dev/null \ "http://${TARGET_IP}:${API_PORT}/api"; then bashio::log.info "deCONZ API is reachable." - break + ready=1 + break fi sleep 1 done -if [ "$i" -eq 30 ]; then +if [ "$ready" -ne 1 ]; then bashio::log.error "deCONZ API is not reachable after 30 seconds – stopping discovery." exit 1 fiAlso applies to: 34-37
🧹 Nitpick comments (5)
deconz/translations/de.yaml (1)
6-15: Unify capitalization of debug labels (optional)Use uppercase acronyms (APS, ZCL, ZDP, OTAU) and title-case “Info” for consistency with typical UI labels.
dbg_aps: - name: Debug aps + name: Debug APS dbg_info: - name: Debug info + name: Debug Info dbg_otau: - name: Debug otau + name: Debug OTAU dbg_zcl: - name: Debug zcl + name: Debug ZCL dbg_zdp: - name: Debug zdp + name: Debug ZDPdeconz/rootfs/etc/services.d/deconz/discovery (4)
20-22: Allow host/port overrides via env for flexibility.
Lets power users/tests point discovery at a different IP/port without editing the script.Apply:
-TARGET_IP="127.0.0.1" -API_PORT="40850" +TARGET_IP="${DECONZ_API_HOST:-127.0.0.1}" +API_PORT="${DECONZ_API_PORT:-40850}"
34-37: Exit code policy: consider exiting 0 to avoid addon restart loops.
If discovery is ancillary, a non-zero exit may cause s6 to keep restarting the service.Option if you prefer non-fatal behavior:
- bashio::log.error "deCONZ API is not reachable after 30 seconds – stopping discovery." - exit 1 + bashio::log.error "deCONZ API is not reachable after 30 seconds – stopping discovery." + exit 0Please confirm intended s6 behavior for this script in this service context.
39-49: Read SQLite DB in read-only mode to avoid locks.
Prevents write-lock contention with deCONZ while querying.Adjust the sqlite3 call (works when the CLI supports URI and mode=ro):
- api_key=$(sqlite3 "${DATA_STORE}" \ + api_key=$(sqlite3 "file:${DATA_STORE}?mode=ro" \ 'SELECT apikey FROM auth WHERE devicetype="Home Assistant" ORDER BY createdate DESC LIMIT 1' 2>/dev/null)If the base image sqlite3 lacks URI support, keep current behavior.
Also applies to: 50-55
71-83: Optional: log deCONZ error reason at debug level during key creation.
Helps diagnose “link button not pressed” vs. other failures without leaking secrets.You can extract and log the error description when no key is returned:
if bashio::var.has_value "${api_key}"; then bashio::log.info "API key successfully created after ${i} seconds." break fi + err="$(bashio::jq "${result}" '.[0].error.description // empty')" + if [ -n "$err" ]; then + bashio::log.debug "API key creation attempt ${i}: ${err}" + fi
📜 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/rootfs/etc/services.d/deconz/discovery(1 hunks)deconz/translations/de.yaml(1 hunks)
🧰 Additional context used
🪛 GitHub Check: YAMLLint
deconz/translations/de.yaml
[failure] 20-20:
trailing spaces
🪛 GitHub Actions: Lint
deconz/translations/de.yaml
[error] 20-20: yamllint: trailing spaces detected at line 20, column 54 (trailing-spaces).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build aarch64 deconz add-on
- GitHub Check: Build armhf deconz add-on
- GitHub Check: Build amd64 deconz add-on
🔇 Additional comments (7)
deconz/rootfs/etc/services.d/deconz/discovery (7)
12-12: LGTM: Clear startup log.
Helpful for tracing discovery lifecycle.
68-89: LGTM: API-key creation hardened (timeouts, content-type, masked logs).
Solid improvements; good retry window and clean logging.
90-106: LGTM: bridge-id retrieval is robust (timeouts + strict hex validation).
Prevents “null”/empty values from being accepted.
107-111: LGTM: graceful exit when bridge ID cannot be obtained.
Avoids noisy failures when deCONZ isn’t ready yet.
115-115: Confirm var.json caret semantics for numeric port.
Caret prefix should keep the value unquoted. Just double-check against current bashio version.
122-122: LGTM: positive confirmation log on successful discovery.
Good operator feedback.
124-124: LGTM: error log on discovery failure.
Appropriate visibility without leaking payload.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deconz/rootfs/etc/services.d/otau-ledvance/run (1)
23-26: Validate required fields and avoid corrupt/partial files.Skip entries with missing metadata and write downloads atomically with curl -f to prevent saving HTML/partial files as firmware.
OSRAM_COMPANY=$( echo "${OSRAM_DATA}" | jq --raw-output ".firmwares[$i].identity.company // empty" 2>/dev/null) OSRAM_PRODUCT=$( echo "${OSRAM_DATA}" | jq --raw-output ".firmwares[$i].identity.product // empty" 2>/dev/null) OTAU_FILENAME=$( echo "${OSRAM_DATA}" | jq --raw-output ".firmwares[$i].name // empty" 2>/dev/null) OTAU_URL="$URL_OSRAM/download/${OSRAM_COMPANY}/${OSRAM_PRODUCT}/latest" + # Skip if any required field is missing + if [ -z "${OSRAM_COMPANY}" ] || [ -z "${OSRAM_PRODUCT}" ] || [ -z "${OTAU_FILENAME}" ]; then + continue + fi @@ - curl -s -L -o "${OTAU_FILE}" "${OTAU_URL}" || true + # Download atomically; only move into place on success + curl -fsSL --retry 3 --retry-delay 2 --max-time 60 -o "${OTAU_FILE}.tmp" "${OTAU_URL}" && \ + mv -f "${OTAU_FILE}.tmp" "${OTAU_FILE}" || true ((DL_DONE+=1))Also applies to: 28-33
♻️ Duplicate comments (1)
deconz/rootfs/etc/services.d/deconz/discovery (1)
23-36: Fix 30th-attempt success being treated as failure (off-by-one).If the API becomes reachable exactly on the 30th attempt, you log success and then immediately treat it as a failure because
$iequals30. Use a readiness flag instead of comparing$i.# Wait until deCONZ API is reachable bashio::log.info "Waiting for deCONZ API to be available..." -for i in $(seq 1 30); do +ready=0 +for i in $(seq 1 30); do if curl -fsS --connect-timeout 1 --max-time 2 -o /dev/null \ "http://${TARGET_IP}:${API_PORT}/api"; then bashio::log.info "deCONZ API is reachable." - break + ready=1 + break fi sleep 1 done -if [ "$i" -eq 30 ]; then +if [ "$ready" -ne 1 ]; then bashio::log.error "deCONZ API is not reachable after 30 seconds – stopping discovery." exit 0 fi
🧹 Nitpick comments (9)
deconz/rootfs/etc/services.d/otau-ledvance/run (2)
20-22: Harden JSON parsing against schema changes/invalid payloads.Make length computation tolerant to missing keys or invalid JSON.
-OSRAM_DATA_SIZE="$(echo "${OSRAM_DATA}" | jq --raw-output '.firmwares | length')" +OSRAM_DATA_SIZE="$( + jq -er '.firmwares // [] | length' <<<"${OSRAM_DATA}" 2>/dev/null || echo 0 +)"
34-38: Use arithmetic test; avoid '[ ... ]' with '=='.This is clearer and shell-agnostic.
-if [ "$((DL_DONE % 10))" == "0" ]; then +if (( DL_DONE % 10 == 0 )); thendeconz/rootfs/usr/share/www/ingress.html (3)
165-181: Duplicate ripple implementations (CSS ::after + JS) cause double flashes. Pick one.Keep the JS ripple (better click-positioned) and remove the CSS pseudo-element.
- .mdc-button::after { - content: ""; - position: absolute; - border-radius: 50%; - width: 100px; - height: 100px; - transform: scale(0); - background: rgba(255,255,255,0.4); - opacity: 0; - transition: transform 0.5s, opacity 1s; - pointer-events: none; - } - .mdc-button:active::after { - transform: scale(1.8); - opacity: 0.6; - transition: 0s; - }Also applies to: 224-238
151-164: Respect reduced-motion preference.Disable ripple/animations for users with prefers-reduced-motion.
@keyframes ripple { to { transform: scale(4); opacity: 0; } } + @media (prefers-reduced-motion: reduce) { + * { animation: none !important; transition: none !important; } + .ripple { display: none !important; } + }Also applies to: 224-238
80-84: Remove hard-coded left offset on title.The 60px left margin looks like a leftover spacer; let toolbar padding handle alignment.
- .toolbar h1 { - margin: 0 0 0 60px; + .toolbar h1 { + margin: 0; font-size: 20px; font-weight: 500; }deconz/rootfs/etc/services.d/deconz/discovery (4)
39-49: Downgrade severity when DB is missing; it's not fatal.Missing DB only skips local lookup; the flow still creates/validates an API key. Use warning to avoid noisy “error” logs for a non-fatal path.
- bashio::log.error "No database found in either path! Proceeding without serial." + bashio::log.warning "No database found in either path; proceeding without serial."
50-55: Guard against missing sqlite3 binary.If sqlite3 isn’t present, this will fail silently. Add an explicit check and log once.
# Read API key from database 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) + if 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) + else + bashio::log.warning "sqlite3 binary not found; skipping DB lookup for API key." + fi fi
73-93: Optional: clarify link-button wait loop messaging.When the POST returns the typical “link button not pressed” error, you silently retry. Consider a single info-level message like “Waiting for Phoscon authentication…” on first failure, then debug logs to avoid user confusion.
95-115: Optional: shorten serial poll interval after first success signal.If config is reachable, polling every 5s for up to ~2 minutes is conservative. You could use a shorter interval (e.g., 2s) for the first 20s to speed up happy-path onboarding.
📜 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 (8)
deconz/CHANGELOG.md(1 hunks)deconz/DOCS.md(6 hunks)deconz/config.yaml(3 hunks)deconz/rootfs/etc/services.d/deconz/discovery(1 hunks)deconz/rootfs/etc/services.d/otau-ledvance/run(2 hunks)deconz/rootfs/usr/share/www/ingress.html(1 hunks)deconz/translations/de.yaml(1 hunks)deconz/translations/en.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- deconz/CHANGELOG.md
- deconz/translations/en.yaml
- deconz/config.yaml
- deconz/DOCS.md
- deconz/translations/de.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build armhf deconz add-on
- GitHub Check: Build aarch64 deconz add-on
🔇 Additional comments (3)
deconz/rootfs/etc/services.d/otau-ledvance/run (1)
5-5: Branding/casing updates look good.Consistent “OSRAM LEDVANCE” usage and improved log wording.
Also applies to: 12-12, 16-16
deconz/rootfs/usr/share/www/ingress.html (2)
8-31: New theming and dark-mode support LGTM.Good use of CSS vars, readable palette, and dark-mode overrides.
Also applies to: 31-55, 56-63
206-215: Ingress copy/links look good.Clear phrasing, correct external link attributes, and concise CTA buttons.
Also applies to: 217-219
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
deconz/rootfs/etc/services.d/otau-ledvance/run (1)
15-16: Hardened curl flags and failure path—LGTM.
Matches the prior recommendation to fail fast and back off.
🧹 Nitpick comments (3)
deconz/rootfs/etc/services.d/otau-ledvance/run (3)
20-21: Make JSON parsing resilient if.firmwaresis missing or JSON is invalid.
Default to empty array and suppress jq errors to avoid noisy logs/empty var pitfalls.-OSRAM_DATA_SIZE="$(echo "${OSRAM_DATA}" | jq --raw-output '.firmwares | length')" +OSRAM_DATA_SIZE="$(jq -r '.firmwares // [] | length' <<<"${OSRAM_DATA}" 2>/dev/null || echo 0)"
26-31: URL-encode path components to avoid 404s on spaces/special chars.
Encodecompanyandproductbefore composing the download URL.- OTAU_URL="$URL_OSRAM/download/${OSRAM_COMPANY}/${OSRAM_PRODUCT}/latest" + OSRAM_COMPANY_ESC="$(printf '%s' "${OSRAM_COMPANY}" | jq -sRr @uri)" + OSRAM_PRODUCT_ESC="$(printf '%s' "${OSRAM_PRODUCT}" | jq -sRr @uri)" + OTAU_URL="${URL_OSRAM}/download/${OSRAM_COMPANY_ESC}/${OSRAM_PRODUCT_ESC}/latest"
42-46: Use arithmetic test for modulo check.
Minor readability/intent improvement.- if [ "$((DL_DONE % 10))" == "0" ]; then + if (( DL_DONE % 10 == 0 )); then
📜 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 (1)
deconz/rootfs/etc/services.d/otau-ledvance/run(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build armhf deconz add-on
- GitHub Check: Build aarch64 deconz add-on
🔇 Additional comments (2)
deconz/rootfs/etc/services.d/otau-ledvance/run (2)
5-5: Branding/casing update looks good.
12-12: Clear startup log message—LGTM.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deconz/rootfs/etc/services.d/otau-ledvance/run (1)
56-60: Fix rate-limit sleep: currently sleeps even when no downloads occurred.
WithDL_DONE=0initially,0 % 10 == 0triggers a 65s sleep on every iteration that skipped download. Gate onDL_DONE > 0.Apply:
- if [ "$((DL_DONE % 10))" == "0" ]; then + if (( DL_DONE > 0 && DL_DONE % 10 == 0 )); then # LEDVANCE/OSRAM API RateLimits : The rate limit 10 calls per 60 seconds or quota 100 MB per month. - DL_DONE=0 - sleep 65 + sleep 65 fiOptional: track API call attempts (success or failure) separately to respect a “10 calls/min” limit even on failures.
🧹 Nitpick comments (2)
deconz/rootfs/etc/services.d/otau-ledvance/run (2)
28-31: Optional: log skip reason for missing fields at debug level.
Helps diagnose catalog anomalies without noisy logs.Example:
- if [ -z "${OSRAM_COMPANY}" ] || [ -z "${OSRAM_PRODUCT}" ] || [ -z "${OTAU_FILENAME}" ]; then - continue - fi + if [ -z "${OSRAM_COMPANY}" ] || [ -z "${OSRAM_PRODUCT}" ] || [ -z "${OTAU_FILENAME}" ]; then + bashio::log.debug "Skipping firmware index ${i}: missing company/product/name." + continue + fi
46-55: Harden downloads; consider retrying on transient errors.
Good atomic move and cleanup. Add retry for all transient errors if supported by curl in base image.Apply:
- if curl -fsSL --retry 3 --retry-delay 2 --max-time 60 -o "${OTAU_FILE}.tmp" "${OTAU_URL}"; then + if curl -fsSL --retry 3 --retry-all-errors --retry-delay 2 --max-time 60 -o "${OTAU_FILE}.tmp" "${OTAU_URL}"; thenPlease confirm
curl --versionsupports--retry-all-errorsin this add-on base. If not, keep current flags.
📜 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 (1)
deconz/rootfs/etc/services.d/otau-ledvance/run(2 hunks)
🔇 Additional comments (2)
deconz/rootfs/etc/services.d/otau-ledvance/run (2)
5-5: Branding/header looks good.
Consistent with other updates in this PR.
12-12: Startup log message LGTM.
Clear and consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
deconz/rootfs/etc/services.d/otau-ledvance/run (3)
29-37: Validate before encoding/building URLMinor: skip empty fields first, then compute encoded segments and OTAU_URL. Avoids doing work (and briefly forming URLs) with empty inputs.
Apply:
- # Encode path segments for safety - OSRAM_COMPANY_ESC="$(jq -rn --arg s "${OSRAM_COMPANY}" '$s|@uri')" - OSRAM_PRODUCT_ESC="$(jq -rn --arg s "${OSRAM_PRODUCT}" '$s|@uri')" - OTAU_URL="${URL_OSRAM}/download/${OSRAM_COMPANY_ESC}/${OSRAM_PRODUCT_ESC}/latest" - - # Skip if any required field is missing - if [ -z "${OSRAM_COMPANY}" ] || [ -z "${OSRAM_PRODUCT}" ] || [ -z "${OTAU_FILENAME}" ]; then - continue - fi + # Skip if any required field is missing + if [ -z "${OSRAM_COMPANY}" ] || [ -z "${OSRAM_PRODUCT}" ] || [ -z "${OTAU_FILENAME}" ]; then + continue + fi + + # Encode path segments for safety (after validation) + OSRAM_COMPANY_ESC="$(jq -rn --arg s "${OSRAM_COMPANY}" '$s|@uri')" + OSRAM_PRODUCT_ESC="$(jq -rn --arg s "${OSRAM_PRODUCT}" '$s|@uri')" + OTAU_URL="${URL_OSRAM}/download/${OSRAM_COMPANY_ESC}/${OSRAM_PRODUCT_ESC}/latest"
39-49: Good sanitization; consider enforcing max filename lengthCurrent basename/vendor/product sanitization looks solid. Optional: clamp final filename to ≤255 bytes to avoid FS edge cases.
Example replacement for the final path assignment:
- OTAU_FILE="/data/otau/${CO_SAN}-${PRD_SAN}-${OTAU_BASENAME}" + PREFIX="${CO_SAN}-${PRD_SAN}-" + MAXLEN=255 + MAX_BASELEN=$(( MAXLEN - ${#PREFIX} )) + if (( ${#OTAU_BASENAME} > MAX_BASELEN )); then + OTAU_BASENAME="${OTAU_BASENAME:0:MAX_BASELEN}" + fi + OTAU_FILE="/data/otau/${PREFIX}${OTAU_BASENAME}"
64-68: Prefer arithmetic test for rate-limit moduloUse (( ... )) and guard against DL_DONE=0 to be explicit.
Apply:
- if [ "$((DL_DONE % 10))" == "0" ]; then + if (( DL_DONE > 0 && DL_DONE % 10 == 0 )); then # LEDVANCE/OSRAM API RateLimits : The rate limit 10 calls per 60 seconds or quota 100 MB per month. DL_DONE=0 sleep 65 fi
📜 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 (1)
deconz/rootfs/etc/services.d/otau-ledvance/run(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build armhf deconz add-on
- GitHub Check: Build aarch64 deconz add-on
🔇 Additional comments (3)
deconz/rootfs/etc/services.d/otau-ledvance/run (3)
15-18: Resilient fetch + backoff: LGTMGood use of curl -fsSL, retries, timeout, and a clear backoff path.
20-23: JSON validation with jq -er: LGTMFailing fast on malformed payloads and backing off is correct.
25-25: Loop bound arithmetic: LGTMUsing OSRAM_DATA_SIZE directly in (( ... )) avoids the previous quoting pitfall.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
deconz/rootfs/etc/services.d/otau-ledvance/run (6)
1-3: Enable strict mode and safe IFS early.Prevents subtle bugs on unset vars/pipelines and word-splitting issues.
# shellcheck shell=bash # ============================================================================== +# Fail fast and use safe word-splitting +set -Eeuo pipefail +IFS=$'\n\t' # Download available firmware update for OSRAM LEDVANCE
12-12: Include add-on version in startup log.Adds useful context in logs.
-bashio::log.info "Running the OSRAM LEDVANCE OTA updater..." +bashio::log.info "Running the OSRAM LEDVANCE OTA updater... (add-on $(bashio::addon.version))"
15-18: Harden curl retries/connect timeout.Covers transient TLS/resolve errors and slow connects.
-if ! OSRAM_DATA="$(curl -fsSL --retry 3 --retry-delay 2 --max-time 30 "${URL_OSRAM}")"; then +if ! OSRAM_DATA="$(curl -fsSL --retry 3 --retry-all-errors --retry-delay 2 --connect-timeout 10 --max-time 30 "${URL_OSRAM}")"; thenNote:
--retry-all-errorsrequires curl ≥ 7.71. If unavailable in the base image, drop that flag.
39-49: Good sanitization; also clamp name lengths to avoid >255-byte filenames.Some vendor/product/filenames can be long; trim parts to remain portable across filesystems.
OTAU_BASENAME="${OTAU_BASENAME//[^A-Za-z0-9._-]/_}" - CO_SAN="${OSRAM_COMPANY//[^A-Za-z0-9._-]/_}" - PRD_SAN="${OSRAM_PRODUCT//[^A-Za-z0-9._-]/_}" + CO_SAN="${OSRAM_COMPANY//[^A-Za-z0-9._-]/_}" + PRD_SAN="${OSRAM_PRODUCT//[^A-Za-z0-9._-]/_}" + # Clamp component lengths to keep final filename <255 bytes + CO_SAN="${CO_SAN:0:32}" + PRD_SAN="${PRD_SAN:0:64}" + OTAU_BASENAME="${OTAU_BASENAME:0:150}" OTAU_FILE="/data/otau/${CO_SAN}-${PRD_SAN}-${OTAU_BASENAME}"
69-73: Rate-limit by attempts, not only successes.The API limits “calls per 60s”; failed attempts still count. Track attempts to avoid 429s.
-DL_DONE=0 +DL_DONE=0 +CALLS=0 ... - if curl -fsSL --retry 3 --retry-delay 2 --max-time 60 -o "${OTAU_FILE}.tmp" "${OTAU_URL}"; then + ((CALLS+=1)) + if curl -fsSL --retry 3 --retry-delay 2 --max-time 60 -o "${OTAU_FILE}.tmp" "${OTAU_URL}"; then if mv -f "${OTAU_FILE}.tmp" "${OTAU_FILE}"; then ((DL_DONE+=1)) ... - if [ "$((DL_DONE % 10))" == "0" ]; then + if [ "$((CALLS % 10))" -eq 0 ] && [ "${CALLS}" -ne 0 ]; then - # LEDVANCE/OSRAM API RateLimits : The rate limit 10 calls per 60 seconds or quota 100 MB per month. - DL_DONE=0 + # LEDVANCE/OSRAM API RateLimits: 10 calls per 60 seconds, 100 MB/month. sleep 65 fi
54-67: Verify LEDVANCE 'sha256' before the atomic move and set explicit file modeFile: deconz/rootfs/etc/services.d/otau-ledvance/run (lines 54–67)
- LEDVANCE Zigbee firmware API provides a per-firmware SHA‑256 in the "sha256" JSON field — compute sha256 of "${OTAU_FILE}.tmp" and compare before mv; on mismatch rm -f "${OTAU_FILE}.tmp" and continue.
- After successful mv, set predictable permissions: chmod 0644 "${OTAU_FILE}" || true.
- if curl -fsSL --retry 3 --retry-delay 2 --max-time 60 -o "${OTAU_FILE}.tmp" "${OTAU_URL}"; then + if curl -fsSL --retry 3 --retry-delay 2 --max-time 60 -o "${OTAU_FILE}.tmp" "${OTAU_URL}"; then + # TODO (optional): verify checksum if provided by API before moving if mv -f "${OTAU_FILE}.tmp" "${OTAU_FILE}"; then + chmod 0644 "${OTAU_FILE}" || true ((DL_DONE+=1))
📜 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 (1)
deconz/rootfs/etc/services.d/otau-ledvance/run(1 hunks)
🔇 Additional comments (4)
deconz/rootfs/etc/services.d/otau-ledvance/run (4)
5-5: LGTM: Branding/title update.
20-23: LGTM: Validate JSON payload and back off on failure.
25-33: LGTM: Loop bounds and URI-encoding.Using arithmetic context for the bound and percent-encoding path segments is correct.
34-37: LGTM: Required-field guard.Prevents malformed entries from progressing.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
deconz/rootfs/etc/services.d/otau-ledvance/run (1)
20-23: JSON validation with jq -er is spot on.This prevents silent no-ops on malformed payloads and backs off correctly.
🧹 Nitpick comments (6)
deconz/rootfs/etc/services.d/otau-ledvance/run (6)
15-17: Add explicit Accept header for robustness.Some APIs vary response by content negotiation. Request JSON explicitly.
-if ! OSRAM_DATA="$(curl -fsSL --retry 3 --retry-all-errors --retry-delay 2 --connect-timeout 10 --max-time 30 "${URL_OSRAM}")"; then +if ! OSRAM_DATA="$(curl -fsSL --retry 3 --retry-all-errors --retry-delay 2 --connect-timeout 10 --max-time 30 \ + -H "Accept: application/json" "${URL_OSRAM}")"; then
29-37: Skip early before URI-encoding to save work and logs.Guard on missing fields before the encode/build step.
- # Encode path segments for safety - OSRAM_COMPANY_ESC="$(jq -rn --arg s "${OSRAM_COMPANY}" '$s|@uri')" - OSRAM_PRODUCT_ESC="$(jq -rn --arg s "${OSRAM_PRODUCT}" '$s|@uri')" - OTAU_URL="${URL_OSRAM}/download/${OSRAM_COMPANY_ESC}/${OSRAM_PRODUCT_ESC}/latest" - - # Skip if any required field is missing - if [ -z "${OSRAM_COMPANY}" ] || [ -z "${OSRAM_PRODUCT}" ] || [ -z "${OTAU_FILENAME}" ]; then - continue - fi + # Skip if any required field is missing + if [ -z "${OSRAM_COMPANY}" ] || [ -z "${OSRAM_PRODUCT}" ] || [ -z "${OTAU_FILENAME}" ]; then + continue + fi + # Encode path segments for safety + OSRAM_COMPANY_ESC="$(jq -rn --arg s "${OSRAM_COMPANY}" '$s|@uri')" + OSRAM_PRODUCT_ESC="$(jq -rn --arg s "${OSRAM_PRODUCT}" '$s|@uri')" + OTAU_URL="${URL_OSRAM}/download/${OSRAM_COMPANY_ESC}/${OSRAM_PRODUCT_ESC}/latest"
39-49: Cap final path length to avoid filesystem max-name issues.Very long company/product/filename combos can exceed 255 bytes. Truncate basename to keep path safe.
OTAU_BASENAME="${OTAU_BASENAME//[^A-Za-z0-9._-]/_}" CO_SAN="${OSRAM_COMPANY//[^A-Za-z0-9._-]/_}" PRD_SAN="${OSRAM_PRODUCT//[^A-Za-z0-9._-]/_}" - OTAU_FILE="/data/otau/${CO_SAN}-${PRD_SAN}-${OTAU_BASENAME}" + OTAU_FILE="/data/otau/${CO_SAN}-${PRD_SAN}-${OTAU_BASENAME}" + # Ensure path length stays well below typical 255-byte limits + MAX_FILEPATH=240 + avail=$(( MAX_FILEPATH - ${#CO_SAN} - ${#PRD_SAN} - 2 )) # 2 hyphens + if [ "${avail}" -lt 1 ]; then avail=1; fi + if [ ${#OTAU_BASENAME} -gt "${avail}" ]; then + OTAU_BASENAME="${OTAU_BASENAME:0:${avail}}" + OTAU_FILE="/data/otau/${CO_SAN}-${PRD_SAN}-${OTAU_BASENAME}" + fi
55-67: Harden download curl like the index fetch and surface errors.Retry all transient errors and set a connect-timeout.
- if curl -fsSL --retry 3 --retry-delay 2 --max-time 60 -o "${OTAU_FILE}.tmp" "${OTAU_URL}"; then + if curl -fsSL --retry 3 --retry-all-errors --retry-delay 2 --connect-timeout 10 --max-time 60 \ + --show-error -o "${OTAU_FILE}.tmp" "${OTAU_URL}"; then
69-73: Rate-limit by requests, not only successful downloads.Failures still hit the API; consider throttling on attempts to respect “10 calls/min”.
Add a requests counter near DL_DONE (outside this hunk):
# near: DL_DONE=0 DL_REQ=0Increment before each attempt and throttle on DL_REQ:
- if (( DL_DONE > 0 && DL_DONE % 10 == 0 )); then + if (( DL_REQ > 0 && DL_REQ % 10 == 0 )); then # LEDVANCE/OSRAM API RateLimits : The rate limit 10 calls per 60 seconds or quota 100 MB per month. - DL_DONE=0 + DL_REQ=0 sleep 65 fiAnd just before the curl call:
((DL_REQ+=1))
1-3: Enable strict mode and cleanup trap (optional hardening).Reduce footguns and clear partials on termination.
Add near the top (outside the shown diff hunk):
set -Eeuo pipefail trap 'rm -f /data/otau/*.tmp 2>/dev/null || true' TERM INT EXITIf the global trap feels too broad, track the current tmp file in a var and remove only that on EXIT.
📜 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 (1)
deconz/rootfs/etc/services.d/otau-ledvance/run(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build armhf deconz add-on
- GitHub Check: Build aarch64 deconz add-on
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
deconz/rootfs/usr/share/www/ingress.html (3)
80-84: Remove leftover left margin on toolbar title (layout nit).
margin: 0 0 0 60px;appears to reserve space for a non-existent icon. It misaligns the title on small screens.-.toolbar h1 { - margin: 0 0 0 60px; +.toolbar h1 { + margin: 0;
4-6: Advertise supported color schemes to the UA.Small enhancement: helps form controls and scrollbars match your themes.
<meta charset="utf-8"> <meta name="viewport" content="width=device-width, initial-scale=1"> + <meta name="color-scheme" content="light dark"> <title>deCONZ | dresden elektronik</title>
19-27: Double-check contrast for toolbar and button colors.Please verify WCAG AA contrast for:
- Light toolbar
#009AC7on white text,- Button default
#009AC7on white text,- Dark toolbar
#101E24on--primary-text-color.If any fail, slightly darken the background colors or lighten text.
Also applies to: 43-51, 138-149
📜 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 (1)
deconz/rootfs/usr/share/www/ingress.html(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build aarch64 deconz add-on
- GitHub Check: Build armhf deconz add-on
🔇 Additional comments (1)
deconz/rootfs/usr/share/www/ingress.html (1)
165-168: Good accessibility touch with reduced motion.Honoring
prefers-reduced-motionis spot on.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
deconz/rootfs/usr/share/www/ingress.html (2)
144-151: Add a :focus fallback (plus forced-colors) for broader a11y support.You restored focus with :focus-visible—great. Add a generic :focus fallback for browsers lacking :focus-visible, and support Windows High Contrast.
.mdc-button { @@ - outline: none; + outline: none; } .mdc-button:focus-visible { outline: 2px solid var(--primary-color); outline-offset: 2px; box-shadow: 0 0 0 3px rgba(3,169,244,0.35); } +/* Fallback for browsers without :focus-visible */ +.mdc-button:focus { + outline: 2px solid var(--primary-color); + outline-offset: 2px; + box-shadow: 0 0 0 3px rgba(3,169,244,0.35); +} +/* Remove fallback ring for mouse users when :focus-visible is supported */ +.mdc-button:focus:not(:focus-visible) { + outline: none; + box-shadow: 0 2px 4px rgba(0,0,0,0.08); +} + +@media (forced-colors: active) { + .mdc-button { + forced-color-adjust: none; + background: ButtonFace; + color: ButtonText; + border: 1px solid CanvasText; + } + .mdc-button:hover { background: Highlight; color: HighlightText; } + .mdc-button:focus { outline: 2px solid Highlight; outline-offset: 2px; } +}
217-231: Ripple center fallback: LGTM; add clamping to keep ripple in-bounds.Centering for keyboard events is in place. Clamp x/y to [0, width/height] to avoid off-edge placement on atypical events.
const rect = button.getBoundingClientRect(); const size = Math.max(rect.width, rect.height); ripple.style.width = ripple.style.height = size + 'px'; const isKeyboard = (e.detail === 0 && e.clientX === 0 && e.clientY === 0); - const x = isKeyboard ? rect.width / 2 : (e.clientX - rect.left); - const y = isKeyboard ? rect.height / 2 : (e.clientY - rect.top); + const clamp = (v, min, max) => Math.min(Math.max(v, min), max); + const xRel = isKeyboard ? rect.width / 2 : (e.clientX - rect.left); + const yRel = isKeyboard ? rect.height / 2 : (e.clientY - rect.top); + const x = clamp(xRel, 0, rect.width); + const y = clamp(yRel, 0, rect.height); ripple.style.left = (x - size / 2) + 'px'; ripple.style.top = (y - size / 2) + 'px';
🧹 Nitpick comments (2)
deconz/rootfs/usr/share/www/ingress.html (2)
109-117: Don’t remove underlines from body-text links.Keeping underlines for inline links improves discoverability; restrict “no underline” to the button anchors only.
-a { - text-decoration: none; - color: var(--link-color); - font-weight: 500; - transition: color 0.3s; -} -a:hover { - color: var(--link-hover-color); - text-decoration: none; -} +a { color: var(--link-color); font-weight: 500; transition: color 0.3s; } +a:hover { color: var(--link-hover-color); } +/* Keep underlines for inline/text links */ +.card p a { text-decoration: underline; text-underline-offset: 2px; } +/* Buttons remain without underlines */ .mdc-button { @@ - text-decoration: none; + text-decoration: none; }Also applies to: 125-146
171-174: Scope reduced-motion overrides more narrowly.Disabling all animations/transitions globally can affect UA controls or future content. Limit to ripple and button transitions.
-@media (prefers-reduced-motion: reduce) { - * { animation: none !important; transition: none !important; } - .ripple { display: none !important; } -} +@media (prefers-reduced-motion: reduce) { + .ripple { display: none !important; } + .mdc-button { transition: none !important; } +}
📜 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 (1)
deconz/rootfs/usr/share/www/ingress.html(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build aarch64 deconz add-on
- GitHub Check: Build armhf deconz add-on
🔇 Additional comments (2)
deconz/rootfs/usr/share/www/ingress.html (2)
4-7: Themeing and dark-mode meta: LGTM.Good use of CSS variables, color-scheme meta, and prefers-color-scheme overrides.
Also applies to: 9-31, 32-56
204-207: Good security defaults on external links.target="_blank" with rel="noopener noreferrer" is correct.
|
All errors should now be fixed. |
mattreim
changed the title
You didn't. My original review comment was about:
See: #4161 (review) You haven't done that. See also our contribution guidelines here: https://developers.home-assistant.io/docs/review-process#creating-the-perfect-pr ../Frenck |
|
I've put enough work and time into this PR. The ingress.html has been aligned with the DOCS.md and en.yaml. Only the discovery is a bugfix, but if you don't want to fix it, I can remove it. You have to tell me what you want. |
|
Side-eying this PR and recent comments, from a long-term user perspective without coding skills:
|
|
@bcutter Thank you very much for your detailed comment. As a long-time open source smart home user, I started helping with a few projects about two years ago. Always with the intention of improving stability and giving something back to the community. The deCONZ developers usually also have their own add-ons or programs that they maintain and therefore cannot contribute to all systems. Because I have some experience with deCONZ and also help there, I wanted to contribute to this add-on. I have been testing all the changes in my repository, sometimes for weeks. For this reason, I also reset the otau-ledvance script because some firmware files were no longer downloading. The rest runs very well and reliably on my main system. |
|
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. Give something back to the community ;) @bcutter And thank you again, it really shows that with such large projects the individual building blocks are important in order to keep it alive or even reactivate it. |
The problem is this PR does multiple things, I'm not against you fixing multiple things. But a PR should do 1 thing. Please split it up in multiple PRs. I wouldn't expect an improvement, bugfix, and dependency bump to ever sit in a single contribution. ../Frenck |
mattreim
changed the title
mattreim
changed the title
|
@frenck I have now broken it down into three PRs.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
deconz/CHANGELOG.md (1)
5-7: Clarify the 8.3.0 bullet wordingThese release notes read less direct than prior entries. Consider tightening the phrasing so each bullet leads with the goal. For example:
-- Ingress entry page revised (for example, light and dark mode) -- Update DOCS.md and a few corrections +- Add light and dark mode support to the ingress entry page +- Refresh DOCS.md and apply related corrections
📜 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 (1)
deconz/CHANGELOG.md(1 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
mattreim
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
deconz/DOCS.md (3)
31-33: Switch to imperative tone for hardware path guidance.Documentation guidelines call for direct instructions. Reword to give a clear action and eliminate “may”.
As per coding guidelines
-If you're using Home Assistant you may find the correct value for this on the -**Settings** -> **System** -> **Hardware** -> **All hardware** page. It is recommended -to use a "by-id" path to the device if one exists, as it is not subject to -change if other devices are added to the system. +In Home Assistant, go to **Settings** -> **System** -> **Hardware** -> **All hardware** to find the device path. +Use a "by-id" path when available because it does not change when you add other hardware.
43-44: Use direct instruction for web UI entry points.Keep the tone instructional by leading with the location and removing “you can”.
As per coding guidelines
-After installing and launching this add-on, you can select **deCONZ** under web UI to access -the deCONZ GUI via VNC, or select **Phoscon** to configure the settings in the Phoscon App. +In the add-on web UI, select **deCONZ** to access the deCONZ GUI via VNC, or select **Phoscon** to configure settings in the Phoscon App.
160-161: Tighten wording in device path example.Avoid “above mentioned”; prefer clearer Microsoft Style phrasing.
As per coding guidelines
- (and similar for RaspBee and the original ConBee, replace `XXXXXXXX` with the value you see in your above mentioned hardware page) + (and similar for RaspBee and the original ConBee; replace `XXXXXXXX` with the value shown on that hardware page)
📜 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 (1)
deconz/DOCS.md(7 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/DOCS.md
🪛 LanguageTool
deconz/DOCS.md
[grammar] ~31-~31: There might be a mistake here.
Context: ...All hardware page. It is recommended to use a "by-id" path to the device if o...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...e if one exists, as it is not subject to change if other devices are added to the...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ..., "/dev/ttyAMA0", or "/dev/ttyACM0". 2. Click on "SAVE" to save the add-on confi...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...select deCONZ under web UI to access the deCONZ GUI via VNC, or select **Phos...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ...s add-on, back up your configuration via the Phoscon App and restore it after ins...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...se the following settings: - Host: core-deconz - (API) Port: 40850 - **WebSocket Port...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...Host**: core-deconz - (API) Port: 40850 - WebSocket Port: 8081 _Note: The abo...
(QB_NEW_EN)
[grammar] ~154-~154: There might be a mistake here.
Context: ...All hardware page. It is recommended to use a "by-id" path to the device if o...
(QB_NEW_EN)
[grammar] ~155-~155: There might be a mistake here.
Context: ...e if one exists, as it is not subject to change if other devices are added to the...
(QB_NEW_EN)
[grammar] ~161-~161: Use a hyphen to join words.
Context: ...XXwith the value you see in your above mentioned hardware page) -"/dev/ttyUSB...
(QB_NEW_EN_HYPHEN)
[grammar] ~182-~182: There might be a mistake here.
Context: ...ckup and download it onto your computer. 2. Uninstall the add-on. 3. In Home Assista...
(QB_NEW_EN)
[style] ~186-~186: This sentence contains multiple usages of the word “again”. Consider removing or replacing it.
Context: ...e deCONZ add-on again, and configure it again according to the [instructions](#config...
(REPETITION_OF_AGAIN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build amd64 deconz add-on
- GitHub Check: Build armhf deconz add-on
- GitHub Check: Build aarch64 deconz add-on
mattreim
changed the title
There was a problem hiding this comment.
This is still a bit mixed bag with DOCS.md updates, naming corrections and ingress page changes, but since its mainly ingress, I am fine with it.
However, for some reason the ingress page changes doesn't seem to look the same is in the screenshot. Also dark mode seems not to work. I restarted the add-on with the change and double checked the ingress.html file, it matches what this PR provides. Did maybe some changes got lost in the course of this PR?
It's still the old page, but after clearing your browser cache, the new one should appear.. To check it, I looked at it again with HTML Online Viewer and it works as expected. |
|
Hm, I realized that Ctrl+F5 does not force a refresh of iframes, so the browser simply showed the cached version. Right clicking inside the ingress iframe and refreshing the frame specifically made the new page load (this is available in Firefox as well as in Chrome) 👍 . It seems to me that browser still should request the content more aggressively, but maybe the ingress proxy causes some issues here. But anyhow, all this is unrelated to this PR. |
5 participants
----Light mode----
----Dark mode----
The DOCS.md has been adapted to the current circumstances.
This manufacturer is written in lowercase dresden elektronik. The integration has already been adjusted #150355.
Expand English tanslation
Summary by CodeRabbit
New Features
Documentation
Style
Chores