Fix scheduling: week bitfield, enable/disable convention, CLI improvements by eman · Pull Request #77 · eman/nwp500-python

eman · 2026-02-17T20:31:07Z

Summary

Fixes critical bugs in reservation and TOU scheduling code:

Fixed Issues

Week Bitfield Encoding (CRITICAL)
- Fixed MGPP week bitfield to match NaviLink APK protocol
- Sunday is now correctly bit 7 (128), Monday bit 6 (64), ..., Saturday bit 1 (2)
- Bit 0 is unused
- Was inverted in our implementation; verified against reference captures
Enable/Disable Convention
- Fixed to use standard device boolean: 1=OFF, 2=ON (was inverted)
- Applies to reservation entries (enable) and global schedule (reservationUse)
- Global reservation status now correctly shows DISABLED when reservationUse=1
- Aligns with other device binary sensors and app behavior
Reservation Set Command Timeout
- Fixed subscription pattern with extra wildcards preventing response matching
- Command now receives confirmations correctly
Intermittent Fetch Bug
- Tightened MQTT topic filter for reservation fetch: /res/ → /res/rsv/ with content validation
- Prevents false matches on unrelated response messages

Added Features

CLI anti-legionella set-period: New subcommand to change cycle period (1-30 days) without toggling the feature

Verification

Verified against device MQTT captures (week=62, 124, 130)
Verified against app behavior (schedules now match)
Verified against reference/WEEK_BITFIELD_ENCODING.md (KDEnum.MgppReservationWeek from NaviLink APK)
All 437 tests pass, CI lint and mypy clean

These files were accidentally included in a Python project and are not needed. The project uses pip and pyproject.toml for dependency management. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

.agent is CLI tool metadata/configuration and should not be committed to the repository. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Added: .mypy_cache, .ruff_cache, .bandit, .agent These are tool-generated caches and configuration that should not be committed to the repository. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…n, CLI improvements - Fixed MGPP week bitfield: Sunday=bit7(128), Mon=bit6(64),...,Sat=bit1(2), bit0 unused (was inverted; protocol verified against NaviLink APK reference) - Fixed enable/disable: 1=OFF, 2=ON (standard device boolean, was inverted) Affects reservation/TOU enable flags and global reservationUse - Fixed 'reservations set' command timeout due to wildcard pattern mismatch - Tightened MQTT topic filter for reservation fetch: /res/ → /res/rsv/ + content check - Added 'anti-legionella set-period' CLI command (1-30 days) - Updated all tests, docs, and models to use correct encoding/convention - All 437 tests pass, CI lint and mypy clean Verified against: - Device MQTT captures (week=62, week=124, week=130 match protocol) - App behavior (schedules now match) - reference/WEEK_BITFIELD_ENCODING.md (KDEnum.MgppReservationWeek) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This pull request fixes critical bugs in reservation and TOU scheduling code, specifically correcting the week bitfield encoding to match the NaviLink APK protocol and fixing the enable/disable convention for device boolean values. The PR also adds CLI improvements including new anti-legionella subcommands and enhanced reservation management.

Changes:

Fixed MGPP week bitfield encoding (Sunday=bit7, Monday=bit6, ..., Saturday=bit1, bit0 unused)
Fixed enable/disable convention to use standard device boolean (1=OFF, 2=ON)
Fixed MQTT topic filtering for reservation responses
Added new Pydantic models (ReservationEntry, ReservationSchedule) for type-safe handling
Added CLI commands for reservation management (add, delete, update)
Added CLI anti-legionella commands (enable, disable, status, set-period)

Reviewed changes

Copilot reviewed 21 out of 23 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
src/nwp500/encoding.py	Updated week bitfield encoding to correct MGPP protocol values, fixed enable/disable convention in build_reservation_entry
src/nwp500/models.py	Added ReservationEntry and ReservationSchedule Pydantic models with computed fields for display values
src/nwp500/mqtt/control.py	Fixed enable/disable convention in update_reservations and configure_tou_schedule
src/nwp500/cli/handlers.py	Refactored reservation handling with shared _fetch_reservations helper, added handlers for add/delete/update/anti-legionella commands
src/nwp500/cli/main.py	Added CLI commands for reservation management and anti-legionella, added auto-detection of unit system
src/nwp500/cli/rich_output.py	Updated reservation table formatting to include mode and unit-aware temperature display
tests/test_models.py	Added comprehensive tests for ReservationEntry and ReservationSchedule models
tests/test_api_helpers.py	Updated tests to verify correct week bitfield encoding and enable/disable convention
docs/guides/scheduling.rst	New comprehensive scheduling guide consolidating reservations, TOU, vacation, and anti-legionella documentation
docs/python_api/cli.rst	Updated CLI documentation with new reservation and anti-legionella commands
docs/python_api/device_control.rst	Fixed documentation cross-references
docs/guides/time_of_use.rst	Updated week bitfield documentation with correct bit values
docs/index.rst	Updated documentation structure
CHANGELOG.rst	Added version 7.5.1 entry documenting all fixes and additions
.gitignore	Added tool cache directories
package.json, package-lock.json	Removed unnecessary Node.js files
docs/guides/scheduling_features.rst, docs/guides/reservations.rst	Removed and consolidated into scheduling.rst
.agent/workflows/pre-completion-testing.md	Removed agent workflow documentation

Copilot · 2026-02-17T20:41:57Z

docs/python_api/cli.rst

+
+   nwp-cli anti-legionella enable --period <days>
+   nwp-cli anti-legionella disable
+   nwp-cli anti-legionella status


The set-period subcommand is missing from the CLI documentation. According to the PR description, a new anti-legionella set-period subcommand was added. The documentation should be updated to include this command in the list of subcommands and provide usage examples.

Copilot · 2026-02-17T20:41:57Z

docs/guides/scheduling.rst

+       "week": 31,           # Bitfield for weekdays
+                              # (same as reservations)
+                              # 31 = Mon-Fri


The comment states "31 = Mon-Fri" for the week bitfield, but according to the corrected MGPP week bitfield encoding (Monday=64, Tuesday=32, Wednesday=16, Thursday=8, Friday=4), Mon-Fri should be 64 + 32 + 16 + 8 + 4 = 124, not 31. This needs to be updated to reflect the corrected encoding.

Suggested change

"week": 31, # Bitfield for weekdays

# (same as reservations)

# 31 = Mon-Fri

"week": 124, # Bitfield for weekdays

# (same as reservations)

# 124 = Mon-Fri

Copilot · 2026-02-17T20:41:58Z

docs/guides/scheduling.rst

+   # Off-peak: 9 PM – 9 AM weekdays
+   off_peak = {
+       "season": 224, "week": 31,
+       "startHour": 21, "startMinute": 0,
+       "endHour": 9, "endMinute": 0,
+       "priceMin": 8, "priceMax": 10, "decimalPoint": 2
+   }
+
+   # Shoulder: 9 AM – 2 PM weekdays
+   shoulder = {
+       "season": 224, "week": 31,
+       "startHour": 9, "startMinute": 0,
+       "endHour": 14, "endMinute": 0,
+       "priceMin": 12, "priceMax": 18, "decimalPoint": 2
+   }
+
+   # Peak: 2 PM – 9 PM weekdays
+   peak = {
+       "season": 224, "week": 31,
+       "startHour": 14, "startMinute": 0,
+       "endHour": 21, "endMinute": 0,
+       "priceMin": 20, "priceMax": 35, "decimalPoint": 2
+   }


All three TOU period examples use "week": 31 to represent weekdays, but according to the corrected MGPP week bitfield encoding, weekdays (Mon-Fri) should be 124, not 31. The week values in these examples need to be updated to 124.

Copilot · 2026-02-17T20:41:58Z

src/nwp500/cli/__main__.py

+    """Set Anti-Legionella cycle period in days (1-30)."""
+    await handlers.handle_enable_anti_legionella_request(mqtt, device, days)


The set-period subcommand calls handle_enable_anti_legionella_request, which sends the ANTI_LEGIONELLA_ON command. This means it will always enable anti-legionella, even if it was previously disabled. According to the PR description, this command should "change the cycle period (1-30 days) without toggling the feature". To achieve this behavior, either:

Create a separate handler that first checks the current anti-legionella status and only sends the command if it's already enabled, OR

Update the PR description to clarify that set-period enables anti-legionella if disabled

The current implementation doesn't match the stated goal of "without toggling the feature".

Copilot · 2026-02-17T20:41:58Z

CHANGELOG.rst

+
+Added
+-----
+- **CLI ``anti-legionella set-period``**: New subcommand to change the Anti-Legionella cycle period (1-30 days) without toggling the feature. Use ``nwp-cli anti-legionella set-period 7`` to update cycle period.


The CHANGELOG states that the new CLI command changes the period "without toggling the feature", but the implementation always calls enable_anti_legionella, which enables anti-legionella even if it was previously disabled. Either the implementation needs to be fixed to preserve the current enable/disable state, or the CHANGELOG should be updated to accurately describe the behavior (e.g., "New subcommand to set Anti-Legionella cycle period, enabling the feature if disabled").

Copilot · 2026-02-17T20:41:59Z

src/nwp500/encoding.py

        ...     temperature=140.0
        ... )
-        {'enable': 1, 'week': 42, 'hour': 6, 'min': 30, 'mode': 3, 'param': 120}
+        {'enable': 1, 'week': 21, 'hour': 6, 'min': 30, 'mode': 3, 'param': 120}


The example output shows 'enable': 1 for enabled=True, but the correct implementation returns 'enable': 2 for True (device boolean convention: 2=enabled, 1=disabled). The example output should show 'enable': 2 to match the actual function behavior.

Suggested change

{'enable': 1, 'week': 21, 'hour': 6, 'min': 30, 'mode': 3, 'param': 120}

{'enable': 2, 'week': 21, 'hour': 6, 'min': 30, 'mode': 3, 'param': 120}

Copilot · 2026-02-17T20:41:59Z

docs/guides/scheduling.rst

+   from nwp500 import ReservationEntry
+
+   entry = ReservationEntry(
+       enable=1, week=62, hour=6, min=30, mode=4, param=120


The example shows enable=1 resulting in entry.enabled # True, but according to the device boolean convention documented at lines 217-218 (and implemented in the code), enable=1 means disabled and enable=2 means enabled. This example should use enable=2 to correctly demonstrate an enabled reservation.

Suggested change

enable=1, week=62, hour=6, min=30, mode=4, param=120

enable=2, week=62, hour=6, min=30, mode=4, param=120

Copilot · 2026-02-17T20:41:59Z

docs/guides/scheduling.rst

+       mode_id=4,
+       temperature=60.0   # In user's preferred unit
+   )
+   # {'enable': 1, 'week': 31, 'hour': 6, 'min': 30,


The example output shows 'enable': 1 for enabled=True, but the correct implementation returns 'enable': 2 for True (device boolean convention: 2=enabled, 1=disabled). The comment should show 'enable': 2 to match the actual function behavior.

Suggested change

# {'enable': 1, 'week': 31, 'hour': 6, 'min': 30,

# {'enable': 2, 'week': 31, 'hour': 6, 'min': 30,

Copilot · 2026-02-17T20:42:00Z

docs/guides/scheduling.rst

+   encode_week_bitfield(["MO", "TU", "WE", "TH", "FR"])
+   # 31


The example shows encode_week_bitfield(["MO", "TU", "WE", "TH", "FR"]) returning 31, but according to the new week bitfield encoding (Monday=64, Tuesday=32, Wednesday=16, Thursday=8, Friday=4), the correct value should be 64 + 32 + 16 + 8 + 4 = 124, not 31. This needs to be updated to reflect the corrected MGPP week bitfield encoding.

Copilot · 2026-02-17T20:42:00Z

docs/guides/scheduling.rst

+   # 31
+
+   encode_week_bitfield([5, 6])   # Saturday + Sunday
+   # 96


The example shows encode_week_bitfield([5, 6]) (Saturday + Sunday with 0-based Monday indexing) returning 96, but according to the new week bitfield encoding (Saturday=2, Sunday=128), the correct value should be 2 + 128 = 130, not 96. This needs to be updated to reflect the corrected MGPP week bitfield encoding.

Suggested change

# 96

# 130

…tation - Fixed docstring inconsistencies in encoding.py (2=enabled, 1=disabled) - Fixed week bitfield example values (31→124, 96→3) - Fixed enable flag example values (1→2) - Added anti-legionella set-period command with state preservation - Added documentation for new anti-legionella set-period CLI command - All 436 tests passing, linting clean Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The docstring shows [5, 6] (Saturday + Sunday) should encode to 130 (2 + 128), not 3 or 96. Corrected to match the actual encoding function output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Replace standalone bandit tool with ruff's flake8-bandit plugin: - Added 'S' (security) rules to ruff lint configuration - Configured per-file ignores for test/example credentials (S105, S106) - Added test assertion ignores (S101) and bare except ignores (S110) - Removed separate tox bandit environment - Updated CI workflow to run security checks via ruff instead of tox - All 436 tests passing, linting clean Benefits: - Unified linting tool (ruff handles all checks) - Simplified CI/CD pipeline - Faster security scanning as part of normal lint job Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The bandit configuration is now handled directly in pyproject.toml via ruff's flake8-bandit plugin. The .bandit file is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Move 7.5.0 and 7.5.1 entries to 'Unreleased' section since no git tags exist for these versions. This aligns with Keep a Changelog format and clarifies that these changes haven't been released yet. Current released version is still 7.4.6. The Unreleased section accumulates changes until a formal release (with git tag) is made. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

eman and others added 5 commits February 17, 2026 09:08

Remove Node.js package files (package.json, package-lock.json)

f11c613

These files were accidentally included in a Python project and are not needed. The project uses pip and pyproject.toml for dependency management. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Remove .agent directory from git tracking

d6fb00f

.agent is CLI tool metadata/configuration and should not be committed to the repository. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add tool cache directories to .gitignore

a23dba3

Added: .mypy_cache, .ruff_cache, .bandit, .agent These are tool-generated caches and configuration that should not be committed to the repository. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

reservation fixes

fbfacd0

eman requested a review from Copilot February 17, 2026 20:35

Copilot started reviewing on behalf of eman February 17, 2026 20:36 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

eman and others added 6 commits February 17, 2026 12:47

Fix week bitfield example in docs (96→130)

252c844

The docstring shows [5, 6] (Saturday + Sunday) should encode to 130 (2 + 128), not 3 or 96. Corrected to match the actual encoding function output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Remove .bandit config file (replaced by ruff configuration)

5ea3128

The bandit configuration is now handled directly in pyproject.toml via ruff's flake8-bandit plugin. The .bandit file is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Update changelog for version 7.4.7 release

d892f28

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

eman temporarily deployed to pypi February 17, 2026 21:16 — with GitHub Actions Inactive

eman merged commit 328925d into main Feb 17, 2026
11 checks passed

eman deleted the reservations branch February 17, 2026 21:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix scheduling: week bitfield, enable/disable convention, CLI improvements#77

Fix scheduling: week bitfield, enable/disable convention, CLI improvements#77
eman merged 11 commits intomainfrom
reservations

eman commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		"""Set Anti-Legionella cycle period in days (1-30)."""
		await handlers.handle_enable_anti_legionella_request(mqtt, device, days)

	{'enable': 1, 'week': 21, 'hour': 6, 'min': 30, 'mode': 3, 'param': 120}
	{'enable': 2, 'week': 21, 'hour': 6, 'min': 30, 'mode': 3, 'param': 120}

	enable=1, week=62, hour=6, min=30, mode=4, param=120
	enable=2, week=62, hour=6, min=30, mode=4, param=120

	# {'enable': 1, 'week': 31, 'hour': 6, 'min': 30,
	# {'enable': 2, 'week': 31, 'hour': 6, 'min': 30,

Conversation

eman commented Feb 17, 2026

Summary

Fixed Issues

Added Features

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants