Skip to content

mqtt: expose limit-battery-charge mode in Home Assistant discovery#338

Merged
MaStr merged 1 commit intoMaStr:mainfrom
filiplajszczak:mqtt-mode-discovery-limit-charge
Apr 16, 2026
Merged

mqtt: expose limit-battery-charge mode in Home Assistant discovery#338
MaStr merged 1 commit intoMaStr:mainfrom
filiplajszczak:mqtt-mode-discovery-limit-charge

Conversation

@filiplajszczak
Copy link
Copy Markdown
Contributor

batcontrol already supports mode 8 (Limit Battery Charge), but the MQTT discovery select only exposed the other three modes.

This adds the missing mode to the discovery templates and select options, with a small regression test.

@MaStr MaStr requested a review from Copilot April 16, 2026 17:46
@MaStr
Copy link
Copy Markdown
Owner

MaStr commented Apr 16, 2026

Ups!
Thank you, good catch!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Home Assistant MQTT discovery for batcontrol to include the already-supported mode 8 (“Limit Battery Charge”) in the discovered select entity, and adds a regression test to ensure the discovery templates/options include that mode.

Changes:

  • Add mode 8 (“Limit Battery Charge”) to the MQTT discovery select’s options, value_template, and command_template.
  • Add a regression test validating discovery includes the new mode mapping.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/batcontrol/mqtt_api.py Extends HA MQTT discovery templates/options to expose mode 8 in the mode select.
tests/batcontrol/test_mqtt_api.py Adds a regression test asserting mode 8 is present in discovery options and templates.

Comment on lines +92 to +94
class TestModeDiscovery:
"""Mode discovery should expose the full externally supported mode model."""

Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new discovery-related tests make this module-level scope broader than the current file docstring (which says it focuses on _handle_message bytes decoding). Consider updating the top-level docstring so it reflects both the message-handling and discovery regression coverage.

Copilot uses AI. Check for mistakes.
Comment on lines 756 to 762
options=[
"Charge from Grid",
"Avoid Discharge",
"Limit Battery Charge",
"Discharge Allowed"],
value_template=val_templ,
command_template=cmd_templ)
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

send_mqtt_discovery_for_mode passes a list for options, but publish_mqtt_discovery_message currently annotates options as str (and treats it generically). To avoid confusion and improve static analysis, update the options parameter type to a sequence/list of strings (or None) and keep the formatting consistent (e.g., trailing comma / closing bracket on its own line).

Copilot uses AI. Check for mistakes.
@MaStr MaStr merged commit 251b6f6 into MaStr:main Apr 16, 2026
14 checks passed
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