Conversation
Add comprehensive documentation covering: - Low-level MQTT client methods (update_reservations, request_reservations) - High-level CLI helpers (reservations add/update/delete/get) - Read-modify-write pattern implementation - Why CLI helpers are recommended for typical use cases - Example code for both SDK and CLI approaches This clarifies that the CLI provides the primary public interface for managing reservations programmatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Show three levels of abstraction: 1. Low-level update_reservations() method (exists) 2. CLI helpers (exist) 3. Library helpers needed - add_reservation(), update_reservation(), delete_reservation(), get_reservations() Remove prescriptive language about what should be primary interface.
Extract add_reservation(), delete_reservation(), update_reservation(), and fetch_reservations() from the CLI into a new public module (src/nwp500/reservations.py). - New module raises ValueError/TimeoutError instead of logging errors, which is appropriate for a library API - All four functions are exported from nwp500.__init__ and listed in __all__ - CLI handlers now delegate to the library functions; all inline logic removed - docs/guides/scheduling.rst: replace the 'Library Helpers (Needed)' stub with a real documented section showing all four functions with examples
Contributor
There was a problem hiding this comment.
Pull request overview
Extracts reservation schedule CRUD logic from the CLI into a reusable, public library module so non-CLI consumers can fetch and mutate device reservations via MQTT.
Changes:
- Added
src/nwp500/reservations.pywith async helpers for fetch/add/delete/update using a read-modify-write pattern. - Updated CLI reservation handlers to delegate to the new library helpers.
- Exported the helpers from
nwp500.__init__and expanded scheduling documentation with usage examples.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/nwp500/reservations.py | New public async API for reservation schedule CRUD over MQTT. |
| src/nwp500/cli/handlers.py | Replaces inline reservation logic with calls into the new library module. |
| src/nwp500/init.py | Re-exports the reservation helpers as part of the public package surface. |
| docs/guides/scheduling.rst | Documents low-level and new high-level reservation helper usage. |
- fetch_reservations: use specific response topic instead of wildcard, unsubscribe in finally block, restore previous unit system after parsing - fetch_reservations: remove error logging on timeout (returns None silently) - update_reservation: add range validation for hour/minute/mode when provided - docs: remove non-existent path= kwarg from subscribe_device_feature call - docs: clarify that fetch_reservations returns None on timeout while mutating helpers raise TimeoutError - tests: add test_reservations.py covering fetch success/timeout, add/delete/update success paths, index and range validation, and timeout propagation for all three mutating helpers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts the single-entry reservation CRUD helpers from the CLI into a new public module
src/nwp500/reservations.py, making them available to all library users.Changes
New:
src/nwp500/reservations.pyFour public async functions:
fetch_reservations(mqtt, device)— fetch the full schedule; returnsNoneon timeoutadd_reservation(mqtt, device, *, enabled, days, hour, minute, mode, temperature)— append a new entrydelete_reservation(mqtt, device, index)— remove an entry by 1-based indexupdate_reservation(mqtt, device, index, *, ...)— partial in-place update (only supplied fields change)Functions raise
ValueErrorfor out-of-range arguments andTimeoutError(mutating helpers) when the device doesn't respond.fetch_reservationsreturnsNoneon timeout.Additional improvements over the original CLI logic:
cmd/{type}/{client_id}/res/rsv/rd) instead of a wildcard patternfinallyblock to prevent subscription leakshour/minute/modein bothadd_reservationandupdate_reservationsrc/nwp500/__init__.pyAll four functions exported and listed in
__all__.src/nwp500/cli/handlers.pyThe three handler functions now delegate to the library. All inline logic, the private
_fetch_reservationswrapper, and the_RAW_RESERVATION_FIELDSconstant have been removed.docs/guides/scheduling.rstReplaced the "Library Helpers (Needed)" stub with a real documented section showing all four functions with usage examples.
tests/test_reservations.py(new)19 tests covering:
fetch_reservations: success, timeout, wrong-topic filteringadd_reservation: success, invalid hour/minute/mode, timeoutdelete_reservation: success, disables-when-empty, invalid index, timeoutupdate_reservation: temperature change, field preservation, invalid index/hour/minute/mode, timeout