Skip to content

Add MCP server and duration-based override system for batcontrol#311

Open
MaStr wants to merge 13 commits intomainfrom
claude/evaluate-mcp-server-Qk5i5
Open

Add MCP server and duration-based override system for batcontrol#311
MaStr wants to merge 13 commits intomainfrom
claude/evaluate-mcp-server-Qk5i5

Conversation

@MaStr
Copy link
Copy Markdown
Owner

@MaStr MaStr commented Mar 17, 2026

Summary

This PR adds a Model Context Protocol (MCP) server to batcontrol, enabling AI assistants and other MCP clients to query system state, forecasts, and battery information, as well as manage manual overrides. It also replaces the single-shot api_overwrite flag with a proper duration-based override manager that persists across evaluation cycles.

Key Changes

1. Duration-Based Override Manager (override_manager.py)

  • New OverrideManager class replaces the single-shot api_overwrite boolean flag
  • OverrideState dataclass tracks mode, charge rate, duration, reason, and expiration time
  • Overrides automatically expire after their configured duration
  • Thread-safe implementation with locking for concurrent access
  • Backward compatible: MQTT mode/set now uses the override manager with a configurable default duration (30 minutes)

2. MCP Server Implementation (mcp_server.py)

  • New BatcontrolMcpServer class exposes batcontrol as AI-accessible tools
  • Read tools (9 total):
    • get_system_status: Current mode, SOC, charge rate, override status
    • get_price_forecast: Electricity prices for next 24-48 hours
    • get_solar_forecast: PV production forecast
    • get_consumption_forecast: Household consumption forecast
    • get_net_consumption_forecast: Grid dependency (consumption - production)
    • get_battery_info: Detailed battery state and capacity info
    • get_decision_explanation: Human-readable reasoning for current mode
    • get_configuration: Runtime parameters (thresholds, limits, offsets)
    • get_override_status: Active override details
  • Write tools (4 total):
    • set_mode_override: Override mode for N minutes with reason
    • clear_mode_override: Cancel active override, resume autonomous logic
    • set_charge_rate: Force charge at specified rate for N minutes
    • set_parameter: Adjust runtime configuration parameters
  • Supports both Streamable HTTP (primary, for Docker/HA addon) and stdio (secondary, for local development) transports
  • Runs in-process as a thread alongside main evaluation loop

3. Core Integration (core.py)

  • Initialize OverrideManager on startup
  • Replace api_overwrite flag checks with override_manager.get_override() calls
  • New _apply_override() method applies override mode/charge_rate to inverter
  • Initialize MCP server if enabled in config
  • Publish override status to MQTT (override_active, override_remaining_minutes, override_duration)
  • New MQTT callbacks for override_duration/set and clear_override/set

4. Decision Explainability (logic/default.py, logic_interface.py)

  • Add explanation: List[str] field to CalculationOutput dataclass
  • New _explain() helper method for logic engine to append reasoning steps
  • Logic engine now documents its decision-making process for AI consumption

5. Configuration & Deployment

  • Add mcp section to batcontrol_config_dummy.yaml with transport, host, port options
  • Expose port 8081 in Dockerfile for MCP HTTP server
  • Add --mcp-stdio CLI flag in __main__.py for stdio transport mode
  • Add mcp>=1.0 dependency to pyproject.toml

6. Testing

  • New test_override_manager.py: Unit tests for override state and manager
  • New test_mcp_server.py: Tests for MCP tool registration and execution
  • Updated test_core.py: Verify override manager integration

Notable Implementation Details

  • Thread Safety: Override manager uses a lock to protect concurrent access from main loop and MCP/MQTT threads
  • Forecast Formatting: Helper function _format_forecast_array() converts numpy arrays to structured JSON with timestamps
  • Backward Compatibility: MQTT mode/set continues to work, now using override manager with 30-minute default duration

https://claude.ai/code/session_0145EbJrBDema8V6xSTGdL8M

claude added 3 commits March 17, 2026 18:26
Comprehensive plan for adding a Model Context Protocol (MCP) server to
batcontrol, covering:

- Phase 1: Duration-based OverrideManager to replace the single-shot
  api_overwrite flag (benefits both MCP and existing MQTT API)
- Phase 2: MCP server with read tools (status, forecasts, battery info,
  decision explanation) and write tools (mode override, charge rate)
- Phase 3: Decision explainability in the logic engine
- Phase 4: Home Assistant addon integration (port exposure, config)
- Phase 5: Testing and documentation

Architecture: in-process HTTP server (Streamable HTTP transport) sharing
the Batcontrol instance, with stdio as secondary transport option.

https://claude.ai/code/session_0145EbJrBDema8V6xSTGdL8M
Phase 1 - OverrideManager (replaces single-shot api_overwrite):
- New OverrideManager class with time-bounded overrides that persist
  across evaluation cycles and auto-expire
- MQTT mode/set now creates 30-minute overrides (backward compatible)
- Override status published to MQTT (override_active, remaining_minutes)
- HA auto-discovery messages for override sensors

Phase 2 - MCP Server:
- 9 read tools: get_system_status, get_price_forecast, get_solar_forecast,
  get_consumption_forecast, get_net_consumption_forecast, get_battery_info,
  get_decision_explanation, get_configuration, get_override_status
- 4 write tools: set_mode_override (with duration), clear_mode_override,
  set_charge_rate, set_parameter
- Streamable HTTP transport (port 8081) for Docker/HA addon
- stdio transport via --mcp-stdio flag for local AI tool integration
- In-process daemon thread, same pattern as MqttApi

Phase 3 - Decision Explainability:
- CalculationOutput.explanation field accumulates reasoning steps
- DefaultLogic annotates price analysis, energy balance, and decisions
- Exposed via get_decision_explanation MCP tool

Config & deployment:
- New mcp section in batcontrol_config_dummy.yaml
- mcp>=1.0 added to pyproject.toml dependencies
- EXPOSE 8081 added to Dockerfile
- --mcp-stdio CLI flag added to __main__.py

Tests: 37 new tests (14 override, 23 MCP), all 361 tests pass.

https://claude.ai/code/session_0145EbJrBDema8V6xSTGdL8M
New MQTT endpoints:
- override_duration/set: Pre-configure how long the next mode/set or
  charge_rate/set override will last (1-1440 min, 0 resets to default 30).
  Works like limit_battery_charge_rate/set: set the parameter first,
  then it takes effect on the next mode/set call. This avoids changing
  the existing MQTT message format (backward compatible).
- clear_override/set: Cancel any active override immediately, resuming
  autonomous control at the next evaluation cycle.

Published topics:
- override_duration: Shows the currently configured duration
- HA auto-discovery for override_duration as a number entity (config category)

This gives HA users the same override duration control that the MCP server
already has via the duration_minutes parameter.

https://claude.ai/code/session_0145EbJrBDema8V6xSTGdL8M
Copilot AI review requested due to automatic review settings March 17, 2026 18:52
Three focused docs under docs/:
- mcp-server.md: Architecture, tool reference, config, mode constants
- override-manager.md: API, thread safety, MQTT topics, HA discovery, usage flows
- decision-explainability.md: Data flow, explanation points, lifecycle, example output

These are designed to be AI-consumable (structured tables, code blocks,
concrete examples) for onboarding future contributors or AI agents.

https://claude.ai/code/session_0145EbJrBDema8V6xSTGdL8M
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

Adds an MCP (Model Context Protocol) server and a duration-based override system to batcontrol, enabling external clients (AI tools, MQTT users) to query system state/forecasts and apply manual, time-bounded control overrides across evaluation cycles.

Changes:

  • Introduces OverrideManager/OverrideState to replace the single-shot api_overwrite mechanism with expiring, thread-safe overrides.
  • Adds an in-process MCP server (BatcontrolMcpServer) exposing read/write tools for status, forecasts, decision explanation, and overrides (HTTP + stdio).
  • Extends logic output with explanation steps and integrates override status + controls into core + MQTT.

Reviewed changes

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

Show a summary per file
File Description
todo.md Adds an implementation plan / design notes for MCP + overrides.
src/batcontrol/override_manager.py New duration-based override manager and state dataclass.
src/batcontrol/mcp_server.py New MCP server implementation (tools + HTTP/stdin transports).
src/batcontrol/core.py Integrates override manager + MCP server startup; publishes override status to MQTT; adds decision explanation getter.
src/batcontrol/mqtt_api.py Publishes override-related MQTT topics and adds HA discovery entities for override status/duration.
src/batcontrol/logic/logic_interface.py Adds explanation: List[str] to CalculationOutput.
src/batcontrol/logic/default.py Appends human-readable explanation steps during decision-making.
src/batcontrol/main.py Adds --mcp-stdio CLI path to run MCP over stdio.
config/batcontrol_config_dummy.yaml Adds MCP configuration section (enabled/transport/host/port).
pyproject.toml Adds mcp>=1.0 dependency.
Dockerfile Exposes port 8081 for MCP HTTP server.
tests/batcontrol/test_override_manager.py Adds unit tests for override manager/state.
tests/batcontrol/test_mcp_server.py Adds tests for tool registration and basic tool behavior.
tests/batcontrol/test_core.py Updates tests to assert override manager activation instead of api_overwrite.

claude and others added 2 commits March 17, 2026 19:04
The MCP SDK requires Python >=3.10, but batcontrol supports >=3.9
(needed for HA addon environments that may run Python 3.9).

Changes:
- Move mcp from hard dependency to optional: pip install batcontrol[mcp]
- mcp_server.py: try/except import with MCP_AVAILABLE flag and
  is_available() helper; BatcontrolMcpServer.__init__ raises ImportError
  if mcp package is missing
- core.py: check mcp_module.is_available() before instantiation,
  log warning if enabled but unavailable
- __main__.py: --mcp-stdio exits with error 1 if mcp unavailable
- test_mcp_server.py: pytest.skip(allow_module_level=True) when
  mcp is not installed
- Dockerfile: install with [mcp] extra (Python 3.13 supports it)
- docs/mcp-server.md: document optional dependency and install command

On Python <3.10: batcontrol works normally, MCP features are simply
unavailable, a warning is logged if mcp.enabled=true in config.

https://claude.ai/code/session_0145EbJrBDema8V6xSTGdL8M
- mcp_server.py: remove unused json/time imports
- mcp_server.py: add digits param to _format_forecast_array; price
  forecast now uses digits=4 to preserve EUR/kWh precision
- mcp_server.py: fix duration error message "between 0" → "between 1"
- mcp_server.py: document shutdown() limitation (no FastMCP stop API)
- core.py: default MCP host to 127.0.0.1 (safer; 0.0.0.0 requires
  explicit opt-in in config)
- core.py: reject charge_rate <= 0 in api_set_charge_rate (previously
  charge_rate=0 would silently start charging at default 500W)
- __main__.py: disable core HTTP MCP startup when --mcp-stdio is used
  to prevent two MCP servers running simultaneously
- test_mcp_server.py: remove unused patch/PropertyMock/OverrideState
- test_override_manager.py: remove unused patch import
- config/batcontrol_config_dummy.yaml: update example host to 127.0.0.1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 17, 2026 20:16
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 introduces an MCP (Model Context Protocol) server for Batcontrol and replaces the previous single-cycle api_overwrite flag with a duration-based, thread-safe override system that persists across evaluation cycles.

Changes:

  • Added a new OverrideManager/OverrideState implementation and integrated it into core.py, MQTT callbacks, and status publishing.
  • Implemented an optional MCP server (FastMCP) exposing read/write tools for status, forecasts, decision explanations, overrides, and runtime parameter tuning.
  • Added decision explainability support by extending CalculationOutput and annotating the default logic with explanation steps, plus new tests and docs.

Reviewed changes

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

Show a summary per file
File Description
tests/batcontrol/test_override_manager.py Adds unit tests for override state/manager behavior (expiry, replacement, thread-safety).
tests/batcontrol/test_mcp_server.py Adds MCP tool registration/execution tests (skipped when MCP SDK unavailable).
tests/batcontrol/test_core.py Updates core test to assert override manager activation instead of api_overwrite.
src/batcontrol/override_manager.py New duration-based override manager replacing single-shot overwrite behavior.
src/batcontrol/mqtt_api.py Publishes override status/duration and registers HA discovery entities for override sensors/number.
src/batcontrol/mcp_server.py New MCP server with read/write tools, forecast formatting helper, HTTP/stdin transports.
src/batcontrol/logic/logic_interface.py Extends CalculationOutput with explanation: List[str].
src/batcontrol/logic/default.py Adds _explain() and populates explanation steps throughout decision logic.
src/batcontrol/core.py Integrates OverrideManager, adds _apply_override(), MCP startup, MQTT override callbacks, explanation getter.
src/batcontrol/__main__.py Adds --mcp-stdio flag and stdio-mode MCP server execution path.
pyproject.toml Adds optional dependency group mcp.
docs/override-manager.md Documents override manager design, API, MQTT topics, and HA discovery entities.
docs/mcp-server.md Documents MCP server architecture, configuration, and tool reference.
docs/decision-explainability.md Documents explanation data flow and example output for decision explainability.
config/batcontrol_config_dummy.yaml Adds example mcp config section.
Dockerfile Exposes MCP port and attempts to install wheel with MCP extras in the runtime image.

- mcp_server.py: set_parameter now validates ranges before calling the
  API handler, returning a proper error instead of false success when
  values are out of range
- mcp_server.py: fix _format_forecast_array docstring to say
  {slot, time_start, value} instead of {time, value}
- core.py: downgrade MODE_LIMIT_BATTERY_CHARGE_RATE missing-limit log
  from warning to debug (emitted every eval cycle, not actionable)
- pyproject.toml: add python_version>='3.10' marker to mcp extra so
  pip install batcontrol[mcp] on Python 3.9 does not attempt to
  install the incompatible mcp package
- Dockerfile: install wheel and mcp>=1.0 as separate pip calls;
  appending [mcp] to a wheel filename wildcard is not valid pip syntax

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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 introduces a duration-based manual override system (replacing the prior one-shot api_overwrite flag) and adds an optional MCP (Model Context Protocol) server that exposes batcontrol state/forecasts and control actions as AI-accessible tools.

Changes:

  • Added OverrideManager / OverrideState for time-bounded overrides, integrated into core.py + MQTT publishing of override status.
  • Added optional MCP server (mcp_server.py) with read/write tools, plus a --mcp-stdio CLI mode and optional dependency wiring.
  • Added decision explainability plumbing (CalculationOutput.explanation) and annotated default logic with explanation steps; added unit tests and docs.

Reviewed changes

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

Show a summary per file
File Description
tests/batcontrol/test_override_manager.py New unit tests for override manager behavior and expiry.
tests/batcontrol/test_mcp_server.py New tests validating MCP tool wiring and behavior (currently coupled to MCP internals).
tests/batcontrol/test_core.py Updated assertion for override integration (uses override manager instead of api_overwrite).
src/batcontrol/override_manager.py New duration-based override state + manager with locking and auto-expiry.
src/batcontrol/mqtt_api.py Publishes/discovers new MQTT entities for override status and override duration configuration.
src/batcontrol/mcp_server.py New optional MCP server exposing batcontrol tools (read + write) over HTTP/stdio.
src/batcontrol/logic/logic_interface.py Adds explanation list to calculation output to support explainability.
src/batcontrol/logic/default.py Adds _explain() and appends human-readable reasoning throughout decision logic.
src/batcontrol/core.py Integrates OverrideManager, applies overrides in the main loop, adds MQTT callbacks, and exposes decision explanations; optional MCP server startup/shutdown wiring.
src/batcontrol/main.py Adds --mcp-stdio mode and disables HTTP MCP startup when using stdio.
pyproject.toml Adds optional extra mcp dependency gated to Python >= 3.10.
docs/override-manager.md New design/API documentation for override manager + MQTT contract.
docs/mcp-server.md New architecture/configuration guide for MCP server and available tools.
docs/decision-explainability.md New documentation for explanation data flow and example output.
config/batcontrol_config_dummy.yaml Adds mcp: config section to the dummy config template.
Dockerfile Installs MCP dependency in the container image and exposes port 8081.

- Fix FastMCP.run() called with invalid host/port kwargs; configure via
  mcp.settings.host/port before calling run() (real functional bug)
- Add api_apply_override() public method to Batcontrol, removing
  protected-access violations in mcp_server.py
- Add duration_minutes validation in api_set_mode() and
  api_set_charge_rate() to guard against ValueError from
  override_manager (prevents MQTT callback crashes)
- Fix __main__.py import order (stdlib before local), add module and
  function docstrings, break long lines
- Replace % string formatting with f-strings in mcp_server.py
- Rename PARAM_VALIDATORS to param_validators (snake_case)
- Fix default MCP HTTP bind from 0.0.0.0 to 127.0.0.1
- Use bc.api_get_limit_battery_charge_rate() instead of private
  _limit_battery_charge_rate access in get_configuration tool
- All 361 tests pass; pylint 9.46/10 overall (10.00/10 on new files)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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 introduces a duration-based manual override system and adds an optional MCP (Model Context Protocol) server so external MCP clients (e.g., AI assistants) can query batcontrol status/forecasts and set overrides/parameters.

Changes:

  • Added OverrideManager + OverrideState to replace the one-shot api_overwrite flag with time-bounded overrides.
  • Implemented an optional in-process MCP server (HTTP or stdio via CLI) with read/write tools for status, forecasts, overrides, and runtime parameter updates.
  • Added decision explainability by recording reasoning steps in CalculationOutput.explanation, plus tests/docs/config/deployment updates.

Reviewed changes

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

Show a summary per file
File Description
src/batcontrol/override_manager.py New thread-safe, duration-based override state/manager.
src/batcontrol/mcp_server.py New MCP server exposing batcontrol read/write tools.
src/batcontrol/core.py Integrates override manager, override application, MQTT publishing, and optional MCP startup.
src/batcontrol/mqtt_api.py Publishes/discovers new MQTT entities for override status/duration + new set callbacks.
src/batcontrol/logic/logic_interface.py Extends CalculationOutput with explanation list.
src/batcontrol/logic/default.py Adds _explain() calls to document decision-making steps.
src/batcontrol/__main__.py Adds --mcp-stdio flag and prevents HTTP MCP startup when using stdio mode.
tests/batcontrol/test_override_manager.py Unit tests for override manager/state behavior.
tests/batcontrol/test_mcp_server.py MCP tool registration/execution tests (skipped if mcp not installed).
tests/batcontrol/test_core.py Updates assertion for override-manager-based mode override.
pyproject.toml Adds optional extra mcp dependency with Python>=3.10 marker.
Dockerfile Installs MCP dependency and exposes port 8081.
config/batcontrol_config_dummy.yaml Adds mcp: configuration section.
docs/override-manager.md Documents override manager behavior, MQTT topics, and contract.
docs/mcp-server.md Documents MCP server architecture, tools, configuration, and usage.
docs/decision-explainability.md Documents explanation data flow and example output.

MaStr and others added 2 commits April 2, 2026 16:03
…tests

- core.py: log a warning when mcp.transport is not 'http' so users
  aren't silently left with an unstarted server (fixes silent no-op
  for transport: stdio in config without --mcp-stdio flag)
- mcp_server.py: add 'production_offset' key to get_configuration()
  so clients can round-trip values with set_parameter() (which uses
  'production_offset' as the parameter name, not 'production_offset_percent')
- mcp_server.py: fall back to time.time() when last_run_time is 0
  (unset before first evaluation) to avoid epoch-anchored timestamps
  in all four forecast tools
- test_mcp_server.py: remove unused MODE_NAMES import
- test_core.py: add TestOverrideDurationAndClear with 7 tests covering
  api_set_override_duration() (valid, reset-to-default, reject negative,
  reject >1440) and api_clear_override() (removes override, idempotent,
  publishes MQTT when api present)

All 368 tests pass; pylint 9.46/10.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
override_manager.py:
- Use None sentinel for expires_at instead of 0.0 float equality check
  (M2: float equality is fragile; Optional[float] is unambiguous)
- Snapshot time.time() once in to_dict() so remaining_minutes and
  is_active are guaranteed internally consistent (M3: TOCTOU fix)

mcp_server.py:
- clear_mode_override now calls bc.api_clear_override() instead of
  bc.override_manager.clear_override() directly, so MQTT publishes
  override_active=False immediately and the API audit log is written
  (M1+m2: stale MQTT state + missing log line)
- Remove duplicate production_offset_percent from get_configuration();
  keep production_offset as the single canonical key that matches
  set_parameter() (m1/n3)
- set_charge_rate returns a 'note' field when the requested rate was
  clamped by inverter limits, so clients see the effective vs requested
  discrepancy (m4)
- Remove spurious pylint: disable=protected-access on public method call (n1)

core.py:
- Fix misleading _apply_override log for mode 8: message now correctly
  describes that limit_battery_charge_rate() will switch to
  allow-discharging, not that this method falls back (C1)
- Align duration_minutes validation to < 1 (not <= 0) in api_set_mode
  and api_set_charge_rate, matching api_set_override_duration (m3)

__main__.py:
- Copy mcp config section instead of mutating the shared dict stored in
  bc.config when --mcp-stdio disables HTTP startup (m5)

tests:
- Wire mock_bc.api_clear_override to the real override_manager in
  test_mcp_server.py so clear_mode_override test remains valid
- Add TestApiSetModeAndChargeDuration: 5 tests for explicit duration,
  invalid duration rejection, and None→mqtt_default fallback (m7)

All 373 tests pass; pylint 9.46/10.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 2, 2026 14:17
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

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

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

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

…timestamps

override_manager.py:
- get_override() now returns a defensive copy via dataclasses.replace()
  instead of the live internal OverrideState instance; callers (including
  other threads) can no longer mutate expires_at/charge_rate without
  holding the manager lock, preserving the thread-safety guarantee

mqtt_api.py:
- publish_override_duration() changed from :.0f to :.1f so that
  fractional-minute durations (e.g. 45.5) are published accurately
  instead of being silently rounded to the nearest whole minute

mcp_server.py:
- _format_forecast_array() now uses integer floor division for base_time
  (int(run_time) // interval_seconds * interval_seconds) and casts each
  time_start to int, eliminating floating-point drift in JSON timestamps

All 373 tests pass; pylint 9.46/10.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@MaStr MaStr requested a review from Copilot April 2, 2026 17:08
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

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

- /min_price_difference/set: set minimum price difference in EUR
- /production_offset/set: set production offset percentage (0.0-2.0)
- /override_duration/set: set override duration in minutes (1-1440, 0=reset to default 30 min)
- /clear_override/set: clear active override (any value triggers the clear)
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

The module docstring says /clear_override/set clears the override with “any value”, but the callback is registered with convert=int (core.py), so non-integer payloads (e.g., "true") will be rejected and the clear won’t run. Either update the documentation to match the actual requirement (integer payload), or register the callback with a converter that accepts any payload (e.g., lambda _payload: 0).

Suggested change
- /clear_override/set: clear active override (any value triggers the clear)
- /clear_override/set: clear active override (any integer payload triggers the clear)

Copilot uses AI. Check for mistakes.
MaStr and others added 2 commits April 2, 2026 19:20
core.py:
- Register clear_override/set MQTT callback with str converter instead
  of int; any payload (numeric, "true", empty string) now triggers the
  clear without failing conversion in _handle_message
- Update api_clear_override signature to accept str (matches converter)

override_manager.py:
- Add VALID_OVERRIDE_MODES = {-1, 0, 8, 10} constant
- set_override() now raises ValueError for unknown modes before creating
  the OverrideState; prevents the run() loop from blocking on an override
  that _apply_override() would silently ignore

test_override_manager.py:
- Add test_invalid_mode_raises to cover the new validation

All 374 tests pass; pylint 9.46/10.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix api_set_charge_rate() type hint: float = None -> Optional[float] = None
- Extract MODE_* constants to _modes.py to eliminate duplication between
  core.py and mcp_server.py without circular imports
- Change mcp-server.md config example from host 0.0.0.0 to 127.0.0.1
  to avoid exposing unauthenticated write tools on the network
- Update override-manager.md: clear_override/set type is str, not int
- Fix __main__.py null-safety: handle mcp: null in config (isinstance check)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 2, 2026 17:31
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

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

Comment on lines +15 to +21
logger = logging.getLogger(__name__)

# Default duration for MQTT overrides (backward compatible)
DEFAULT_OVERRIDE_DURATION_MINUTES = 30

# Valid inverter modes accepted by the override system
VALID_OVERRIDE_MODES = {-1, 0, 8, 10}
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

VALID_OVERRIDE_MODES duplicates the mode constants now centralized in batcontrol._modes (used by both core.py and mcp_server.py). To prevent future drift, import and reuse VALID_MODES (or the individual MODE_* constants) from ._modes here instead of maintaining a separate set.

Suggested change
logger = logging.getLogger(__name__)
# Default duration for MQTT overrides (backward compatible)
DEFAULT_OVERRIDE_DURATION_MINUTES = 30
# Valid inverter modes accepted by the override system
VALID_OVERRIDE_MODES = {-1, 0, 8, 10}
from ._modes import VALID_MODES as VALID_OVERRIDE_MODES
logger = logging.getLogger(__name__)
# Default duration for MQTT overrides (backward compatible)
DEFAULT_OVERRIDE_DURATION_MINUTES = 30
# Valid inverter modes accepted by the override system (imported from batcontrol._modes)

Copilot uses AI. Check for mistakes.
Comment on lines +80 to +103
def set_override(self, mode: int, duration_minutes: Optional[float] = None,
charge_rate: Optional[int] = None,
reason: str = "") -> OverrideState:
"""Set a time-bounded override.

Args:
mode: Inverter mode (-1, 0, 8, 10)
duration_minutes: How long the override should last.
None uses default_duration_minutes.
charge_rate: Optional charge rate in W (relevant for mode -1)
reason: Human-readable reason for the override

Returns:
The created OverrideState
"""
if mode not in VALID_OVERRIDE_MODES:
raise ValueError(
f"Invalid mode {mode}. Valid modes: {sorted(VALID_OVERRIDE_MODES)}")

if duration_minutes is None:
duration_minutes = self.default_duration_minutes

if duration_minutes <= 0:
raise ValueError("duration_minutes must be positive")
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

set_override() accepts charge_rate for any mode and doesn’t validate it. This can create inconsistent override state (e.g., mode != -1 but charge_rate set) or even propagate negative/zero charge rates into force_charge() if set programmatically. Consider validating that charge_rate is only allowed for MODE_FORCE_CHARGING and must be > 0, otherwise raise ValueError (or coerce to None).

Copilot uses AI. Check for mistakes.
'max_charging_from_grid_limit': bc.max_charging_from_grid_limit,
'min_price_difference': bc.min_price_difference,
'min_price_difference_rel': bc.min_price_difference_rel,
'production_offset': bc.production_offset_percent,
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

get_configuration() returns production_offset but other MCP outputs (e.g., get_solar_forecast) expose the same value as production_offset_percent. To keep the MCP schema consistent (and preserve backward compatibility if clients relied on the earlier *_percent naming), consider returning both keys (e.g., include production_offset_percent as an alias) or standardize on one name across all tools.

Suggested change
'production_offset': bc.production_offset_percent,
'production_offset': bc.production_offset_percent,
'production_offset_percent': bc.production_offset_percent,

Copilot uses AI. Check for mistakes.
Comment on lines +56 to +83
def mock_bc(self):
"""Create a mock Batcontrol instance with all necessary attributes."""
bc = MagicMock()
bc.override_manager = OverrideManager()
bc.last_mode = 10
bc.last_charge_rate = 0
bc.last_SOC = 65.0
bc.last_stored_energy = 5000.0
bc.last_stored_usable_energy = 4500.0
bc.last_max_capacity = 10000.0
bc.last_free_capacity = 5000.0
bc.last_reserved_energy = 1000.0
bc.discharge_blocked = False
bc.last_run_time = 1700000000.0
bc.time_resolution = 60
bc.last_prices = np.array([0.15, 0.20, 0.25, 0.18])
bc.last_production = np.array([500.0, 1000.0, 800.0, 200.0])
bc.last_consumption = np.array([300.0, 400.0, 500.0, 350.0])
bc.last_net_consumption = np.array([-200.0, -600.0, -300.0, 150.0])
bc.production_offset_percent = 1.0
bc.max_charging_from_grid_limit = 0.8
bc.min_price_difference = 0.05
bc.min_price_difference_rel = 0.1
bc._limit_battery_charge_rate = -1
bc.get_always_allow_discharge_limit.return_value = 0.9
bc.api_get_decision_explanation.return_value = [
"Current price: 0.1500, Battery: 5000 Wh stored",
"Decision: Allow discharge"
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

In mock_bc, api_get_limit_battery_charge_rate isn’t configured. Since get_configuration() calls bc.api_get_limit_battery_charge_rate(), the tool currently returns a MagicMock for limit_battery_charge_rate, which isn’t JSON-serializable and can mask schema/type regressions. Set bc.api_get_limit_battery_charge_rate.return_value (e.g., bc._limit_battery_charge_rate) so the test exercises realistic output types.

Copilot uses AI. Check for mistakes.
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