Add support for Venus A and Venus D

[tomquist]

Summary by CodeRabbit

• New Features

  • Added support for Venus A (VNSA) and Venus D (VNSD) device types, enabling users to configure and manage two additional Venus-based energy storage device models.

• Documentation

  • Updated all relevant documentation, README, and user translations (English and German) to include the new Venus device types in device configuration guidance.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR adds support for two new Venus device types—VNSA (Venus A) and VNSD (Venus D)—by extending device type listings across documentation, translations, and device registration code.

Changes

Cohort / File(s)	Summary
Documentation & Device References CHANGELOG.md, README.md, docs/venus.md	Added VNSA and VNSD to device type enumerations in changelog entry, README device types list, and Venus command documentation.
Translations ha_addon/translations/en.yaml, ha_addon/translations/de.yaml	Updated device configuration descriptions to include VNSA-1 and VNSD-1 in the deviceType examples and allowed values list.
Device Registration src/device/venus.ts	Extended the registered Venus deviceTypes array from ['HMG', 'VNSE3'] to ['HMG', 'VNSE3', 'VNSA', 'VNSD'].

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• PR #182: Updates Venus device type registration in src/device/venus.ts to recognize additional device types (VNSE3), sharing similar device registration expansion logic.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add support for Venus A and Venus D' directly matches the main change across all modified files, which is adding VNSA and VNSD device types to the Venus device support.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch claude/add-venus-device-support-Sj1oS

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36faeaa and 2e844ba.

📒 Files selected for processing (1)

• CHANGELOG.md

⏰ 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: test-addon
• GitHub Check: build-addon (linux/arm/v7)
• GitHub Check: build-addon (linux/arm64)

🔇 Additional comments (1)

CHANGELOG.md (1)

4-4: Changelog entry is clear and well-formatted.

The new entry accurately describes the feature, is properly placed in the [Next] section, and follows the established pattern used by similar Venus device support entries. Device names and identifiers are clearly presented.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

hm2mqtt/src/deviceManager.ts

Lines 48 to 52 in 36faeaa

	this.config.devices.forEach(device => {
	const deviceDefinition = getDeviceDefinition(device.deviceType);
	if (!deviceDefinition) {
	logger.warn(`Skipping unknown device type: ${device.deviceType}`);
	return;

Filter unknown devices before topic lookup

The constructor now skips unknown device types but leaves them in this.config.devices (deviceManager.ts:48-52). Later, findDeviceForTopic iterates over this.config.devices and immediately dereferences topics.deviceTopicOld without a null check (same file ~241), so with a mixed config like the new test case the first MQTT message will throw Cannot read properties of undefined for the invalid entry and abort message handling even though valid devices exist. Filtering unknown devices out of the managed list or skipping them in findDeviceForTopic would avoid the crash.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

CHANGELOG.md (1)

2-5: Clarify the "loop" terminology in the changelog.

The changelog entry mentions "Fix loop when all configured devices have invalid/unknown device types," but based on the code changes in src/deviceManager.ts and src/mqttClient.ts, this appears to be adding validation that throws an error early rather than fixing an actual infinite loop. Consider revising to something like:

• "Add validation to fail fast when all configured devices have invalid/unknown device types"

This would more accurately describe the enhancement.

src/deviceManager.ts (1)

272-275: Good defensive check before GCD calculation.

This validation correctly prevents attempting to calculate GCD on an empty array, which would fail. The guard ensures that at least one device has a polling interval configured.

Optional: Consider a more specific error message

The error message "No valid devices configured" could be more specific since this particular check is about polling intervals. A device could be valid but have no polled messages. Consider:

     if (allPollingIntervals.length === 0) {
-      throw new Error('No valid devices configured');
+      throw new Error('No devices with polling intervals configured');
     }

This helps distinguish between "no valid devices at all" (caught in constructor) vs. "valid devices exist but none have polling enabled."

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 269db33 and 36faeaa.

📒 Files selected for processing (10)

• CHANGELOG.md
• README.md
• docs/venus.md
• ha_addon/translations/de.yaml
• ha_addon/translations/en.yaml
• src/device/venus.ts
• src/deviceManager.test.ts
• src/deviceManager.ts
• src/index.ts
• src/mqttClient.ts

🧰 Additional context used

🧬 Code graph analysis (2)

src/deviceManager.ts (1)

src/deviceDefinition.ts (1)

• getDeviceDefinition (196-206)

src/deviceManager.test.ts (1)

src/deviceManager.ts (1)

• DeviceManager (32-295)

⏰ 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). (5)

• GitHub Check: build (linux/arm/v7)
• GitHub Check: build (linux/amd64)
• GitHub Check: build (linux/arm64)
• GitHub Check: build-addon (linux/arm64)
• GitHub Check: build-addon (linux/arm/v7)

🔇 Additional comments (10)

README.md (1)

317-318: LGTM! Device type documentation expansion looks good.

The new Venus device types (VNSA-X and VNSD-X) are documented consistently with the existing format and align with the code changes.

docs/venus.md (1)

81-81: LGTM! Documentation update is clear and comprehensive.

The Venus device type listing now includes all four supported types with helpful examples.

ha_addon/translations/en.yaml (1)

31-31: LGTM! Translation file updated correctly.

The device type examples in the configuration description now include the new Venus types (VNSA-1, VNSD-1).

ha_addon/translations/de.yaml (1)

31-31: LGTM! German translation updated consistently.

The German translation mirrors the English version with the new Venus device types included.

src/deviceManager.test.ts (2)

57-74: LGTM! Test coverage for invalid device types is thorough.

This test correctly verifies that the DeviceManager throws a clear error message when all configured devices have unknown types, ensuring fail-fast behavior.

---

76-105: Excellent test coverage for mixed valid/invalid scenarios.

This test comprehensively verifies the behavior when some devices are valid and others are invalid:

• Invalid devices correctly return undefined topics
• Valid devices correctly return defined topics
• getPollingInterval() works when at least one valid device exists

This ensures robust handling of partial configuration errors.

src/index.ts (1)

326-334: LGTM! Enhanced error logging improves observability.

The structured error handling provides clear, actionable information while keeping stack traces limited to debug mode. This pairs well with the new validation errors that can be thrown during initialization.

src/mqttClient.ts (1)

162-169: Defensive error handling is appropriate here.

This try/catch guard ensures setupPeriodicPolling fails gracefully if polling intervals cannot be determined. While the DeviceManager constructor also validates device configurations, this provides a safety net in case the MqttClient is called in unexpected scenarios or if the initialization order changes in the future.

The clear error message helps with debugging.

src/device/venus.ts (1)

137-137: LGTM! Clean extension of supported Venus device types.

The addition of 'VNSA' and 'VNSD' device types is straightforward and will be handled by the existing runtime info and BMS message processing logic. This change correctly expands device support without modifying any behavior.

src/deviceManager.ts (1)

46-82: Excellent addition of device validation in constructor.

The validation logic correctly ensures at least one valid device is configured before proceeding. By counting devices that successfully return a device definition and throwing an early error if none exist, this prevents the system from initializing in an invalid state. The error message clearly indicates the issue.