Repository navigation
Add support for VNSA and VNSD device types - #122
Conversation
WalkthroughAdds new Venus device types (VNSA, VNSD) and corresponding min-version entries, broadens firmware-compatibility check from VNSE3-only to any VNS-prefixed devices at the same firmware threshold, and adds a short-circuit error handling branch for API response code "4". Also updates release and addon/versioning automation and changelog insertion. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
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 |
- Added VNSA and VNSD to hame-2025 broker min_versions - Both devices now use the 2025 broker with topic encryption - Set min_version to 0 to support all firmware versions - Follows same pattern as VNSE3 Venus device
- Added VNSA firmware 144 to use_remote_topic_id_versions - Added VNSD firmware 142 to use_remote_topic_id_versions - These devices now use the newer CommonHelper.cq encryption method
- Added VNSA devices with firmware >= 144 to CommonHelper.cq encryption - Added VNSD devices with firmware >= 142 to CommonHelper.cq encryption - Removed incorrect use_remote_topic_id_versions entries - The isSupportVid method auto-detects encryption support based on firmware This enables the newer salt-based CommonHelper.cq encryption method for both Venus A and Venus D devices.
- Consolidated VNSE3, VNSA, and VNSD to single VNS prefix check - All Venus series devices now use firmware >= 139 threshold - Simplifies encryption detection for all Venus models
ccaa251 to
af1f604
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/hame_api.ts (1)
190-194: Document the Hame API response code meaning and verify it corresponds to incorrect password.The error handling for code "4" is logically sound, but lacks documentation. The Hame API response codes are not documented anywhere in the codebase. Add a comment explaining what this code represents for future maintainability:
// Code "4" indicates incorrect password from Hame API if (data.code === "4") { throw new Error( `Incorrect password for ${mailbox}. Please double-check the credentials configured for the add-on and try again. (${data.code} - ${data.msg})`, ); }Additionally, confirm this change aligns with the PR scope—it appears unrelated to the recent work on Venus firmware 139 encryption support.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.mdbrokers.jsonhassio-addon/config.yamlrelease.shsrc/hame_api.tssrc/topic.tssrc/types.ts
✅ Files skipped from review due to trivial changes (2)
- CHANGELOG.md
- hassio-addon/config.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- src/topic.ts
- brokers.json
- src/types.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: tomquist
Repo: tomquist/hame-relay PR: 86
File: src/types.ts:6-19
Timestamp: 2025-10-03T08:30:57.007Z
Learning: Device type names in src/types.ts (including VNSE3) are manufacturer-defined and cannot be modified to follow a consistent naming pattern. The codebase must accommodate both HM* prefixed types and manufacturer-specific names like VNSE3.
⏰ 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). (4)
- GitHub Check: build-addon (linux/arm64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build-addon (linux/arm/v7)
- GitHub Check: build (linux/arm/v7)
🔇 Additional comments (3)
release.sh (3)
178-189: LGTM! Clean tracking logic for version updates.The
needs_amendflag correctly tracks whether changes were made to the config file. Usinggit diff --quietto detect actual changes avoids unnecessary amends when the version is already set to 'next'.
205-210: Confirm that amending the merge commit is the intended workflow.The script amends the develop-sync merge commit (line 176) to include version and changelog updates. While this keeps changes atomic, amending a merge commit can cause history divergence if others have already pulled develop. In a controlled CI/CD environment or with proper coordination, this approach is acceptable.
Please confirm:
- Is this script typically run in a controlled environment (CI/CD, single release manager)?
- Are team members aware not to pull develop until the release completes?
- Would creating a separate "prepare for next development" commit be preferable to amending?
191-203: The sed insertion point is correct for the CHANGELOG.md structure.The assumption about the first line containing the changelog title (
# Changelog) is accurate, and the sed command'1a\'correctly inserts the new [Next] section after it. The multiline syntax with backslashes is properly formatted and compatible with macOS. The grep-based condition also prevents duplicate [Next] sections from being inserted.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.