Feature/arnout/correct message count #250
Conversation
WalkthroughThis set of changes is primarily focused on code formatting, stylistic consistency, and minor documentation improvements across the codebase. A few properties and method signatures were renamed or reformatted for clarity, and logging statements were adjusted for readability. No functional logic or core behavior was altered except for a change in how the expected response count is tracked in the sender component, and a concurrency improvement in gathering missing energy logs. Changes
Sequence Diagram(s)sequenceDiagram
participant Node as Node
participant Circle as Circle Node
participant EnergyLog as Energy Log Updater
Note over Circle: get_missing_energy_logs (old)
Circle->>EnergyLog: Await energy_log_update (sequentially)
loop For each missing address
EnergyLog-->>Circle: Return result
end
Note over Circle: get_missing_energy_logs (new)
Circle->>EnergyLog: Gather energy_log_update tasks (concurrently)
EnergyLog-->>Circle: Return all results (as a group)
sequenceDiagram
participant Sender as StickSender
participant Manager as StickConnectionManager
participant Receiver as StickReceiver
Note over Sender: write_request_to_port
Sender->>Manager: Send request
Manager->>Sender: Receive ACCEPT response
Sender->>Sender: Increment expected_responses
Manager->>Receiver: Process messages
Manager->>Manager: queue_depth = expected_responses - processed_messages
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (3.3.7)plugwise_usb/__init__.pyplugwise_usb/connection/__init__.pyplugwise_usb/connection/manager.py
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #250 +/- ##
==========================================
- Coverage 81.13% 81.08% -0.06%
==========================================
Files 36 36
Lines 7502 7506 +4
==========================================
- Hits 6087 6086 -1
- Misses 1415 1420 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
plugwise_usb/__init__.py(7 hunks)plugwise_usb/api.py(0 hunks)plugwise_usb/connection/__init__.py(1 hunks)plugwise_usb/connection/manager.py(1 hunks)plugwise_usb/connection/queue.py(2 hunks)plugwise_usb/connection/receiver.py(1 hunks)plugwise_usb/connection/sender.py(4 hunks)plugwise_usb/constants.py(2 hunks)plugwise_usb/helpers/cache.py(4 hunks)plugwise_usb/helpers/util.py(2 hunks)plugwise_usb/messages/__init__.py(1 hunks)plugwise_usb/messages/properties.py(2 hunks)plugwise_usb/messages/requests.py(2 hunks)plugwise_usb/messages/responses.py(3 hunks)plugwise_usb/network/__init__.py(4 hunks)plugwise_usb/network/cache.py(1 hunks)plugwise_usb/nodes/celsius.py(1 hunks)plugwise_usb/nodes/circle.py(4 hunks)plugwise_usb/nodes/helpers/__init__.py(1 hunks)plugwise_usb/nodes/helpers/counter.py(3 hunks)plugwise_usb/nodes/helpers/pulses.py(3 hunks)plugwise_usb/nodes/helpers/subscription.py(2 hunks)plugwise_usb/nodes/node.py(3 hunks)plugwise_usb/nodes/scan.py(5 hunks)plugwise_usb/nodes/sed.py(1 hunks)plugwise_usb/nodes/sense.py(1 hunks)plugwise_usb/nodes/stealth.py(1 hunks)plugwise_usb/nodes/switch.py(3 hunks)pyproject.toml(2 hunks)tests/stick_test_data.py(1 hunks)tests/test_usb.py(4 hunks)
💤 Files with no reviewable changes (1)
- plugwise_usb/api.py
🧰 Additional context used
🧬 Code Graph Analysis (14)
plugwise_usb/connection/__init__.py (2)
plugwise_usb/messages/requests.py (17)
NodePingRequest(602-632)send(367-376)send(389-398)send(440-442)send(478-487)send(514-523)send(544-555)send(568-577)send(590-599)send(623-632)send(669-678)send(721-730)send(777-786)send(810-819)send(850-859)send(882-891)NodeInfoRequest(894-923)plugwise_usb/exceptions.py (1)
StickError(32-33)
plugwise_usb/nodes/helpers/counter.py (1)
plugwise_usb/nodes/helpers/pulses.py (1)
add_log(418-453)
plugwise_usb/connection/queue.py (1)
plugwise_usb/connection/manager.py (1)
queue_depth(40-47)
plugwise_usb/nodes/switch.py (2)
plugwise_usb/api.py (1)
NodeFeature(39-54)plugwise_usb/messages/responses.py (1)
switch_state(857-859)
plugwise_usb/nodes/helpers/__init__.py (2)
plugwise_usb/nodes/node.py (2)
is_loaded(201-203)mac(228-230)plugwise_usb/exceptions.py (1)
NodeError(24-25)
plugwise_usb/network/cache.py (3)
plugwise_usb/api.py (1)
mac(270-271)plugwise_usb/messages/__init__.py (1)
mac(53-57)plugwise_usb/nodes/node.py (1)
mac(228-230)
plugwise_usb/helpers/cache.py (2)
plugwise_usb/exceptions.py (1)
CacheError(8-9)tests/test_usb.py (3)
makedirs(1540-1545)makedirs(1643-1648)os_path_join(1519-1521)
plugwise_usb/messages/requests.py (1)
plugwise_usb/messages/responses.py (1)
PlugwiseResponse(98-215)
plugwise_usb/connection/manager.py (2)
plugwise_usb/connection/sender.py (1)
expected_responses(48-50)plugwise_usb/connection/receiver.py (1)
processed_messages(142-144)
plugwise_usb/nodes/scan.py (1)
plugwise_usb/messages/responses.py (1)
switch_state(857-859)
plugwise_usb/messages/properties.py (1)
plugwise_usb/exceptions.py (1)
MessageError(20-21)
plugwise_usb/network/__init__.py (3)
plugwise_usb/messages/requests.py (1)
CirclePlusAllowJoiningRequest(456-487)plugwise_usb/nodes/node.py (1)
mac(228-230)plugwise_usb/messages/responses.py (1)
NodeResponseType(56-72)
tests/test_usb.py (4)
plugwise_usb/__init__.py (1)
nodes(95-99)plugwise_usb/network/__init__.py (1)
nodes(138-142)plugwise_usb/api.py (1)
node_info(246-247)plugwise_usb/nodes/node.py (1)
node_info(223-225)
plugwise_usb/messages/responses.py (2)
plugwise_usb/__init__.py (1)
hardware(128-130)plugwise_usb/messages/properties.py (14)
value(76-80)value(96-100)value(125-129)value(159-163)value(187-191)value(207-211)value(251-255)value(276-280)value(301-305)value(330-334)value(359-363)value(380-384)value(403-407)value_set(244-248)
🪛 GitHub Check: codecov/patch
plugwise_usb/connection/queue.py
[warning] 150-150: plugwise_usb/connection/queue.py#L150
Added line #L150 was not covered by tests
plugwise_usb/helpers/cache.py
[warning] 77-77: plugwise_usb/helpers/cache.py#L77
Added line #L77 was not covered by tests
[warning] 85-85: plugwise_usb/helpers/cache.py#L85
Added line #L85 was not covered by tests
plugwise_usb/nodes/node.py
[warning] 514-514: plugwise_usb/nodes/node.py#L514
Added line #L514 was not covered by tests
plugwise_usb/nodes/circle.py
[warning] 451-451: plugwise_usb/nodes/circle.py#L451
Added line #L451 was not covered by tests
[warning] 479-479: plugwise_usb/nodes/circle.py#L479
Added line #L479 was not covered by tests
plugwise_usb/network/__init__.py
[warning] 263-263: plugwise_usb/network/init.py#L263
Added line #L263 was not covered by tests
[warning] 561-561: plugwise_usb/network/init.py#L561
Added line #L561 was not covered by tests
[warning] 565-565: plugwise_usb/network/init.py#L565
Added line #L565 was not covered by tests
plugwise_usb/__init__.py
[warning] 175-175: plugwise_usb/init.py#L175
Added line #L175 was not covered by tests
[warning] 250-250: plugwise_usb/init.py#L250
Added line #L250 was not covered by tests
[warning] 331-331: plugwise_usb/init.py#L331
Added line #L331 was not covered by tests
plugwise_usb/messages/responses.py
[warning] 781-781: plugwise_usb/messages/responses.py#L781
Added line #L781 was not covered by tests
[warning] 785-785: plugwise_usb/messages/responses.py#L785
Added line #L785 was not covered by tests
[warning] 789-789: plugwise_usb/messages/responses.py#L789
Added line #L789 was not covered by tests
🪛 Ruff (0.11.9)
plugwise_usb/helpers/cache.py
83-83: No whitespaces allowed surrounding docstring text
(D210)
🔇 Additional comments (65)
plugwise_usb/nodes/celsius.py (1)
5-5: Skip formatting-only change.
The added blank line after the module docstring improves readability but introduces no behavioral or API changes.plugwise_usb/nodes/stealth.py (1)
2-2: Skip formatting-only change.
Introducing a blank line after the module docstring aligns with the project's style conventions; no functional impact.plugwise_usb/messages/__init__.py (1)
22-22: Skip formatting-only change.
The blank line added between thePriorityenum andPlugwiseMessageclass is purely stylistic and does not affect functionality.plugwise_usb/connection/receiver.py (1)
516-516: Skip formatting-only change.
A trailing blank line was added at the end of the method for consistency with surrounding code style. No behavior was altered.plugwise_usb/constants.py (1)
35-35: Skip formatting-only change.
Removal of a trailing space on theSTICK_TIME_OUTassignment is a whitespace cleanup with no effect on logic.plugwise_usb/messages/properties.py (2)
12-14: LGTM! Improved readability.The multi-line formatting of the
DESERIALIZE_ERRORconstant improves code readability without changing functionality.
397-397: Good simplification.Removing the unnecessary
int()wrapper around0is a good simplification since0is already an integer literal.plugwise_usb/helpers/util.py (2)
2-2: Good formatting improvement.Adding a blank line after the module docstring follows standard Python formatting conventions.
25-25: Consistent type hint formatting.Adding spaces around the union operator (
str | None) improves readability and follows Python typing conventions.plugwise_usb/network/cache.py (1)
37-39: Improved readability with multi-line formatting.The multi-line formatting of the string assignment improves code readability without changing functionality.
pyproject.toml (3)
261-261: Fixed typo in comment.Good correction of the typo in the S319 rule comment from "suspicious-xml-pull-dom-usag" to "suspicious-xml-pull-dom-usage".
280-280: Verify the TRY rule change is intentional.The change from
"TRY302"to"TRY203"modifies which exception handling rule is active. Please confirm this change aligns with the intended linting policy.
TRY203: Remove exception handler; error is immediately re-raisedTRY302(if it exists): Would need verification of what this rule covers
288-290: Added docstring rules to ignore list.Adding
D205(blank line between summary and description) andD401(imperative mood) to the ignore list is consistent with the project's approach to docstring formatting flexibility.plugwise_usb/nodes/scan.py (4)
52-52: Define consistent sensitivity values
The addition ofSENSITIVITY_MEDIUM_VALUE = 30aligns with existing sensitivity constants for motion sensor configuration, improving code clarity and maintainability.
176-180: Improve conditional readability
The multi-line formatting of the cached motion validation condition enhances readability without altering its logic.
362-366: Use actual switch state in update
Switching toresponse.switch_stateinstead of hard-codedTrueensures the motion state update accurately reflects the incoming response.
482-484: Refactor sensitivity mapping formatting
Breaking thesensitivity_map.get(...)call into multiple lines improves readability while preserving behavior.plugwise_usb/nodes/sense.py (1)
119-121: This removes an extraneous blank line before the return statement, which is purely formatting and has no impact on functionality.plugwise_usb/connection/__init__.py (2)
212-213: ConsolidateNodePingRequestinstantiation
The single-line constructor call forNodePingRequestenhances readability by eliminating the previous multi-line invocation without changing behavior.
220-220: ConsolidateNodeInfoRequestinstantiation
Similarly, theNodeInfoRequestinstantiation is now on one line for consistency with other request calls.plugwise_usb/nodes/helpers/__init__.py (1)
28-34: Enhance decorator readability
Adding blank lines around the innerdecoratedfunction improves visual separation in theraise_not_loadeddecorator without modifying its functionality.plugwise_usb/nodes/sed.py (1)
240-242: The removal of extra blank lines around the earlyreturn Falsemakes the code more compact; this is strictly a formatting tweak.tests/stick_test_data.py (1)
63-63: LGTM: Minor formatting improvementsGood formatting fixes - adding proper spacing after comment markers and colons improves code readability and follows Python style conventions.
Also applies to: 67-67
plugwise_usb/nodes/helpers/subscription.py (2)
23-29: Excellent documentation improvementThe detailed docstring clearly explains the purpose and structure of the
_feature_update_subscribersdictionary, making the code more maintainable and easier to understand.
66-68: Good formatting improvementConverting the method call to multi-line format enhances readability while maintaining the same functionality.
plugwise_usb/nodes/switch.py (3)
56-61: Improved readability with multi-line formattingExpanding the
NodeFeaturetuple to multiple lines makes the code easier to read and maintain, especially when reviewing which features are being set up.
97-97: Consistent comment formattingGood spacing standardization for the region end marker.
107-107: Good practice: Added trailing commaThe trailing comma after the second coroutine argument follows Python best practices and makes future additions to the
gathercall cleaner.plugwise_usb/nodes/helpers/pulses.py (2)
10-10: Import ordering improvementReordering the imports for better organization while maintaining the same functionality.
176-176: Good practice: Added trailing commasAdding trailing commas in the logging calls follows Python best practices and makes future parameter additions cleaner.
Also applies to: 378-378
plugwise_usb/nodes/helpers/counter.py (3)
83-88: Formatting-only change:add_pulse_logconditional reflow
The refactored multi-line conditional improves readability without altering functionality.
166-168: Formatting-only change: multi-linePRODUCTION_HOUR.updatecall
Splitting theupdateinvocation across lines is purely stylistic; logic remains intact.
303-305: Formatting-only change: multi-linelast_reset.replacecall
The re-indentation of thereplacecall enhances clarity but doesn’t affect behavior.tests/test_usb.py (3)
593-593: Assertion updated to useis None
Usingis Noneto checkmodel_typeadheres to PEP 8 and is correct.
2547-2547: Assertion updated to useis None
Consistent Pythonic check forNonein node discovery test.
2612-2612: Assertion updated to useis None
Correctly verifiesmodel_typeremainsNonefor the Switch node.plugwise_usb/connection/queue.py (2)
105-106: LGTM: Formatting improvementGood formatting change that improves readability by splitting the logging call into multiple lines with a trailing comma for consistency.
150-152: LGTM: Formatting improvementConsistent formatting change that matches the multi-line logging style used elsewhere in the codebase.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 150-150: plugwise_usb/connection/queue.py#L150
Added line #L150 was not covered by testsplugwise_usb/messages/requests.py (2)
315-317: LGTM: Method signature formatting improvementGood formatting change that improves readability by splitting the method signature into multiple lines.
1269-1269: LGTM: Docstring cleanupGood cleanup of trailing whitespace in the docstring.
plugwise_usb/nodes/circle.py (2)
5-5: LGTM: Import additions for new functionalityThe new imports for
gatherandfloorsupport the functional improvements in this file.Also applies to: 11-11
512-512: LGTM: Formatting improvementGood consolidation of the debug log call from multi-line to single-line for better readability.
plugwise_usb/network/__init__.py (4)
19-19: LGTM: Import reorderingThe import reordering improves alphabetical consistency.
263-265: LGTM: Improved readability with multi-line formattingThe multi-line formatting of the debug log statement improves readability and aligns with the broader code style improvements in this PR.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 263-263: plugwise_usb/network/init.py#L263
Added line #L263 was not covered by tests
537-538: LGTM: Consistent tuple formattingThe multi-line formatting of the tuple improves readability and maintains consistency.
561-567: LGTM: Multi-line formatting improvementsBoth the ValueError message and debug log statement benefit from multi-line formatting for better readability.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 561-561: plugwise_usb/network/init.py#L561
Added line #L561 was not covered by tests
[warning] 565-565: plugwise_usb/network/init.py#L565
Added line #L565 was not covered by testsplugwise_usb/helpers/cache.py (2)
56-58: LGTM: Consistent multi-line formatting for error messagesThe multi-line formatting of CacheError messages improves readability and maintains consistency across the codebase.
Also applies to: 77-79, 85-87, 126-128
159-159: LGTM: Slice notation formattingThe space addition in the slice notation follows a consistent formatting style, though it doesn't affect functionality.
plugwise_usb/messages/responses.py (3)
654-654: LGTM: Improved debug output with hardware substringDisplaying a substring of the hardware identifier (
[4:10]) in the__repr__method provides more focused and readable debug output while maintaining essential information.
781-793: LGTM: Consistent indentation formattingThe indentation adjustments improve code consistency and readability without affecting functionality.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 781-781: plugwise_usb/messages/responses.py#L781
Added line #L781 was not covered by tests
[warning] 785-785: plugwise_usb/messages/responses.py#L785
Added line #L785 was not covered by tests
[warning] 789-789: plugwise_usb/messages/responses.py#L789
Added line #L789 was not covered by tests
859-859: LGTM: Simplified expressionRemoving unnecessary parentheses simplifies the boolean expression while maintaining the same logic.
plugwise_usb/nodes/node.py (3)
52-52: LGTM: Improved visual separationThe blank line addition after the constant improves visual separation and code readability.
339-341: LGTM: Conditional expression formattingThe multi-line conditional expression maintains readability while following consistent formatting patterns.
514-516: LGTM: List formatting consistencyThe multi-line formatting of the
model_infoassignment improves readability and aligns with the broader formatting improvements in this PR.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 514-514: plugwise_usb/nodes/node.py#L514
Added line #L514 was not covered by testsplugwise_usb/connection/sender.py (3)
41-41: LGTM: Improved semantic clarity with renameThe rename from
_processed_msgs/processed_messagesto_expected_responses/expected_responsesbetter reflects the actual purpose of this counter - tracking expected responses rather than processed messages.Also applies to: 48-50
82-84: LGTM: Multi-line formatting for debug logsThe multi-line formatting of debug log statements improves readability and aligns with the consistent formatting improvements throughout this PR.
Also applies to: 111-115, 130-132
124-124:✅ Verification successful
Verify the logic change for counter increment
The counter increment has been moved to only occur when a
StickResponseof typeACCEPTis received. This changes the counting logic from incrementing on every request to only incrementing on accepted requests.Verify this logic change aligns with the queue depth calculation in
plugwise_usb/connection/manager.py:
🏁 Script executed:
#!/bin/bash # Description: Verify the usage of expected_responses in queue depth calculation # Search for usage of expected_responses or queue_depth calculation rg -A 5 -B 5 "expected_responses|queue_depth" # Check manager.py specifically for queue depth calculation ast-grep --pattern 'def queue_depth($_) { $$$ }'Length of output: 4902
Confirmed: counter increment on ACCEPT aligns with queue depth calculation
The
queue_depthproperty inplugwise_usb/connection/manager.pyis defined as:@property def queue_depth(self) -> int: return self._sender.expected_responses - self._receiver.processed_messagesSince
expected_responsesis now only incremented when aStickResponseof typeACCEPTis received, the queue depth correctly represents the number of accepted requests still awaiting their corresponding node responses. No changes are needed.plugwise_usb/__init__.py (6)
22-24: Improved readability with single-line string formattingThe reformatting of the multi-line string concatenation to a single expression improves readability and follows modern Python conventions.
47-49: Consistent string formatting improvementGood improvement in code readability by consolidating the multi-line string concatenation into a single expression.
174-175: Note: Enhanced error message formatting lacks test coverageThe reformatted error condition and message improve readability, but static analysis indicates this line lacks test coverage. This appears to be a pre-existing gap rather than introduced by these changes.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 175-175: plugwise_usb/init.py#L175
Added line #L175 was not covered by tests
250-252: Improved exception formatting with test coverage gapThe single-line SubscriptionError creation is more readable than the previous multi-line format. However, static analysis shows this line lacks test coverage.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 250-250: plugwise_usb/init.py#L250
Added line #L250 was not covered by tests
283-285: Consistent string concatenation improvementsThe conversion of multi-line string concatenations to single expressions enhances code readability and maintainability across the connection error handling.
Also applies to: 292-295
331-331: Error message formatting improvement with test coverage concernThe single-line error message is cleaner and more readable. Static analysis indicates this line lacks test coverage, which may be worth addressing in future test improvements.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 331-331: plugwise_usb/init.py#L331
Added line #L331 was not covered by testsplugwise_usb/connection/manager.py (2)
41-47: Improved queue depth calculation logic and documentationThe updated
queue_depthproperty now correctly calculates the number of expected responses that haven't been processed yet, which is a more accurate representation of the actual queue state. The enhanced docstring clearly explains the purpose and return value.This change aligns with the PR objective of correcting message count logic to apply only to accepted messages.
49-59: Enhanced method documentationThe detailed docstring for
correct_received_messagesgreatly improves code maintainability by clearly explaining the method's purpose, parameters, and return value.
| @@ -474,8 +476,7 @@ | |||
| create_task(self.energy_log_update(address)) | |||
| for address in missing_addresses | |||
| ] | |||
| for task in tasks: | |||
| await task | |||
| await gather(*tasks, return_exceptions=True) | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Excellent improvement: Concurrent task execution
Changing from sequential await to gather(*tasks, return_exceptions=True) significantly improves performance by executing all energy log update tasks concurrently. The return_exceptions=True parameter ensures that failures in individual tasks don't break the entire operation.
However, this change lacks test coverage according to static analysis. Please ensure this concurrent execution path is properly tested:
🏁 Script executed:
#!/bin/bash
# Description: Search for tests covering the concurrent energy log update logic
# Expected: Find test coverage for the gather-based task execution
# Search for tests that might cover the missing energy logs functionality
rg -A 15 -B 5 "missing.*energy.*log" --type py
# Look for tests that use or mock create_task or gather
rg -A 10 -B 5 "(create_task|gather).*energy" --type py
# Check if there are integration tests for energy log collection
fd test_.*\.py --exec rg -l "(energy.*log|log.*energy)" {} \;Length of output: 10118
Excellent improvement: Concurrent task execution
Changing from sequential await to gather(*tasks, return_exceptions=True) significantly improves performance by executing all energy‐log update tasks concurrently. The return_exceptions=True parameter ensures that failures in individual tasks don’t break the entire operation.
However, our search shows existing tests in tests/test_usb.py mock get_missing_energy_logs, but there are currently no tests explicitly covering the concurrent execution path via gather. Please add unit tests to exercise the branch where more than one log address is missing. For example:
• Simulate a PlugwiseCircle instance with log_addresses_missing = [addr1, addr2, …]
• Monkeypatch energy_log_update to record calls and to raise an exception for at least one address
• Invoke await circle.get_missing_energy_logs() and assert:
– energy_log_update was scheduled via create_task for each missing address
– gather awaited all tasks, capturing exceptions rather than failing fast
– Side effects (cache save, power update) occur as expected after concurrent execution
Consider adding these tests in a new file (e.g. tests/test_circle_concurrency.py) or expanding tests/test_usb.py.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 479-479: plugwise_usb/nodes/circle.py#L479
Added line #L479 was not covered by tests
🤖 Prompt for AI Agents
In plugwise_usb/nodes/circle.py at line 479, the change to use gather for
concurrent execution of energy log update tasks lacks test coverage. Add unit
tests that simulate a PlugwiseCircle instance with multiple missing log
addresses, monkeypatch energy_log_update to track calls and raise exceptions for
some addresses, then call get_missing_energy_logs and assert that create_task
schedules all updates, gather awaits all tasks capturing exceptions without
failing fast, and that subsequent side effects like cache saving and power
updates occur correctly. Place these tests in a new file like
tests/test_circle_concurrency.py or extend tests/test_usb.py.
| return os_path_join(os_path_expand_user("~"), CACHE_DIR) | ||
|
|
||
| async def write_cache(self, data: dict[str, str], rewrite: bool = False) -> None: | ||
| """"Save information to cache file.""" | ||
| """ "Save information to cache file.""" |
There was a problem hiding this comment.
Fix docstring formatting issue
The static analysis tool correctly identifies a whitespace issue in the docstring.
Apply this diff to fix the docstring formatting:
- """ "Save information to cache file."""
+ """Save information to cache file."""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| """ "Save information to cache file.""" | |
| """Save information to cache file.""" |
🧰 Tools
🪛 Ruff (0.11.9)
83-83: No whitespaces allowed surrounding docstring text
(D210)
🤖 Prompt for AI Agents
In plugwise_usb/helpers/cache.py at line 83, the docstring has an extra leading
space before the opening quotes. Remove the extra space so the docstring starts
immediately with triple quotes followed by the text, ensuring proper formatting
and compliance with docstring standards.
|
Please see my feedback on going back to the |
bouwew
force-pushed
the
feature/arnout/correct_message_count
branch
3 times, most recently
from
aff5624 to
e848f5b
Compare
bouwew
force-pushed
the
feature/arnout/correct_message_count
branch
2 times, most recently
from
5983104 to
86f2e8d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
plugwise_usb/helpers/cache.py (1)
150-159:⚠️ Potential issueInvalid-line handling prematurely aborts parsing
Breaking out of the loop stops processing any subsequent valid lines.
Switch tocontinueso only the faulty line is skipped, and drop superfluousstr().- data, - str(self._cache_file), + data, + self._cache_file, ) - break + continue current_data[data[:index_separator]] = data[index_separator + 1 :]
♻️ Duplicate comments (1)
plugwise_usb/helpers/cache.py (1)
83-83: Remove stray space/quote in docstringSame issue flagged previously; docstring still violates D210.
- """ "Save information to cache file.""" + """Save information to cache file."""🧰 Tools
🪛 Ruff (0.11.9)
83-83: No whitespaces allowed surrounding docstring text
(D210)
🧹 Nitpick comments (4)
plugwise_usb/helpers/cache.py (4)
56-58: Grammar correction inCacheErrormessage
does not exists→does not exist.- raise CacheError( - f"Unable to initialize caching. Cache folder '{self._root_dir}' does not exists." - ) + raise CacheError( + f"Unable to initialize caching. Cache folder '{self._root_dir}' does not exist." + )
120-121: Misleading line-count & redundantstr()in debug log
len(data)only reflects the new keys, not the total lines written.
Also,loggingwill stringify the value – the explicitstr()call is unnecessary.- "Saved %s lines to cache file %s", str(len(data)), self._cache_file + "Saved %s lines to cache file %s", + len(data_to_write), + self._cache_file,
126-128: Wrong context in error textThe method is
read_cache, but the message says save.- f"Unable to save cache. Initialize cache file '{self._file_name}' first." + f"Unable to read cache. Initialize cache file '{self._file_name}' first."
132-134: Minor grammar fix in debug message
does not exists→does not exist.- "Cache file '%s' does not exists, return empty cache data", + "Cache file '%s' does not exist, returning empty cache data",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
plugwise_usb/__init__.py(6 hunks)plugwise_usb/api.py(0 hunks)plugwise_usb/connection/__init__.py(1 hunks)plugwise_usb/connection/manager.py(1 hunks)plugwise_usb/connection/queue.py(2 hunks)plugwise_usb/connection/receiver.py(1 hunks)plugwise_usb/connection/sender.py(4 hunks)plugwise_usb/constants.py(1 hunks)plugwise_usb/helpers/cache.py(4 hunks)plugwise_usb/helpers/util.py(2 hunks)plugwise_usb/messages/__init__.py(1 hunks)plugwise_usb/messages/properties.py(2 hunks)plugwise_usb/messages/requests.py(2 hunks)plugwise_usb/messages/responses.py(3 hunks)plugwise_usb/network/__init__.py(3 hunks)plugwise_usb/network/cache.py(1 hunks)plugwise_usb/nodes/celsius.py(1 hunks)plugwise_usb/nodes/circle.py(4 hunks)plugwise_usb/nodes/helpers/__init__.py(1 hunks)plugwise_usb/nodes/helpers/counter.py(3 hunks)plugwise_usb/nodes/helpers/pulses.py(3 hunks)plugwise_usb/nodes/helpers/subscription.py(2 hunks)plugwise_usb/nodes/node.py(3 hunks)plugwise_usb/nodes/scan.py(5 hunks)plugwise_usb/nodes/sed.py(1 hunks)plugwise_usb/nodes/stealth.py(1 hunks)plugwise_usb/nodes/switch.py(3 hunks)pyproject.toml(2 hunks)tests/stick_test_data.py(1 hunks)tests/test_usb.py(4 hunks)
💤 Files with no reviewable changes (1)
- plugwise_usb/api.py
✅ Files skipped from review due to trivial changes (3)
- tests/stick_test_data.py
- plugwise_usb/nodes/helpers/pulses.py
- tests/test_usb.py
🚧 Files skipped from review as they are similar to previous changes (25)
- plugwise_usb/messages/init.py
- plugwise_usb/constants.py
- plugwise_usb/connection/init.py
- plugwise_usb/connection/receiver.py
- plugwise_usb/nodes/celsius.py
- plugwise_usb/nodes/helpers/subscription.py
- plugwise_usb/nodes/stealth.py
- plugwise_usb/init.py
- plugwise_usb/nodes/switch.py
- plugwise_usb/nodes/helpers/init.py
- plugwise_usb/connection/queue.py
- plugwise_usb/messages/requests.py
- plugwise_usb/network/init.py
- plugwise_usb/nodes/helpers/counter.py
- plugwise_usb/helpers/util.py
- plugwise_usb/nodes/node.py
- plugwise_usb/network/cache.py
- plugwise_usb/nodes/sed.py
- plugwise_usb/connection/sender.py
- pyproject.toml
- plugwise_usb/connection/manager.py
- plugwise_usb/messages/responses.py
- plugwise_usb/messages/properties.py
- plugwise_usb/nodes/scan.py
- plugwise_usb/nodes/circle.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/helpers/cache.py (2)
plugwise_usb/exceptions.py (1)
CacheError(8-9)tests/test_usb.py (3)
makedirs(1556-1561)makedirs(1659-1664)os_path_join(1535-1537)
🪛 Ruff (0.11.9)
plugwise_usb/helpers/cache.py
83-83: No whitespaces allowed surrounding docstring text
(D210)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check commit
… back to gather instead of loop
bouwew
force-pushed
the
feature/arnout/correct_message_count
branch
from
86f2e8d to
f521fa7
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugwise_usb/helpers/cache.py (1)
83-83: Fix docstring formatting – duplicate of earlier feedbackExtra leading space breaks PEP-257 style and Ruff’s D210 rule.
- """ "Save information to cache file.""" + """Save information to cache file."""🧰 Tools
🪛 Ruff (0.11.9)
83-83: No whitespaces allowed surrounding docstring text
(D210)
🧹 Nitpick comments (5)
plugwise_usb/helpers/cache.py (5)
56-58: Grammar fix in error message
does not exists→does not exist.
Minor but worth correcting for professional log output.- f"Unable to initialize caching. Cache folder '{self._root_dir}' does not exists." + f"Unable to initialize caching. Cache folder '{self._root_dir}' does not exist."
77-79: Grammar fix in error messageSame typo here.
- "Unable to detect writable cache folder based on 'APPDATA' environment variable." + "Unable to detect a writable cache folder based on the 'APPDATA' environment variable."
118-121: Incorrect line count & unnecessarystr()conversion in debug log
len(data)only counts the new keys, not the total lines written, andlogginghandles type conversion.- "Saved %s lines to cache file %s", str(len(data)), self._cache_file + "Saved %s lines to cache file %s", len(data_to_write), self._cache_file
126-128: Error message references the wrong actionThis is in
read_cache, notsave_cache.- f"Unable to save cache. Initialize cache file '{self._file_name}' first." + f"Unable to read cache. Initialize cache file '{self._file_name}' first."
132-134: Minor grammar tweak in log message
does not exists→does not exist.- "Cache file '%s' does not exists, return empty cache data", + "Cache file '%s' does not exist, returning empty cache data",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
plugwise_usb/__init__.py(6 hunks)plugwise_usb/api.py(0 hunks)plugwise_usb/connection/__init__.py(1 hunks)plugwise_usb/connection/manager.py(1 hunks)plugwise_usb/connection/queue.py(2 hunks)plugwise_usb/connection/receiver.py(1 hunks)plugwise_usb/connection/sender.py(4 hunks)plugwise_usb/constants.py(1 hunks)plugwise_usb/helpers/cache.py(4 hunks)plugwise_usb/helpers/util.py(2 hunks)plugwise_usb/messages/__init__.py(1 hunks)plugwise_usb/messages/properties.py(2 hunks)plugwise_usb/messages/requests.py(2 hunks)plugwise_usb/messages/responses.py(3 hunks)plugwise_usb/network/__init__.py(2 hunks)plugwise_usb/network/cache.py(1 hunks)plugwise_usb/nodes/celsius.py(1 hunks)plugwise_usb/nodes/circle.py(4 hunks)plugwise_usb/nodes/helpers/__init__.py(1 hunks)plugwise_usb/nodes/helpers/counter.py(3 hunks)plugwise_usb/nodes/helpers/pulses.py(3 hunks)plugwise_usb/nodes/helpers/subscription.py(2 hunks)plugwise_usb/nodes/node.py(3 hunks)plugwise_usb/nodes/scan.py(5 hunks)plugwise_usb/nodes/sed.py(1 hunks)plugwise_usb/nodes/stealth.py(1 hunks)plugwise_usb/nodes/switch.py(3 hunks)pyproject.toml(2 hunks)tests/stick_test_data.py(1 hunks)tests/test_usb.py(4 hunks)
💤 Files with no reviewable changes (1)
- plugwise_usb/api.py
✅ Files skipped from review due to trivial changes (2)
- plugwise_usb/nodes/helpers/counter.py
- plugwise_usb/network/cache.py
🚧 Files skipped from review as they are similar to previous changes (26)
- plugwise_usb/nodes/celsius.py
- plugwise_usb/nodes/stealth.py
- plugwise_usb/messages/init.py
- plugwise_usb/connection/init.py
- plugwise_usb/constants.py
- plugwise_usb/connection/receiver.py
- plugwise_usb/nodes/helpers/init.py
- plugwise_usb/nodes/switch.py
- plugwise_usb/nodes/sed.py
- plugwise_usb/init.py
- plugwise_usb/nodes/helpers/subscription.py
- plugwise_usb/helpers/util.py
- plugwise_usb/connection/queue.py
- plugwise_usb/messages/requests.py
- plugwise_usb/nodes/circle.py
- plugwise_usb/connection/sender.py
- plugwise_usb/messages/properties.py
- tests/stick_test_data.py
- plugwise_usb/connection/manager.py
- plugwise_usb/nodes/scan.py
- plugwise_usb/messages/responses.py
- plugwise_usb/nodes/helpers/pulses.py
- pyproject.toml
- tests/test_usb.py
- plugwise_usb/network/init.py
- plugwise_usb/nodes/node.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/helpers/cache.py (2)
plugwise_usb/exceptions.py (1)
CacheError(8-9)tests/test_usb.py (3)
makedirs(1551-1556)makedirs(1654-1659)os_path_join(1530-1532)
🪛 Ruff (0.11.9)
plugwise_usb/helpers/cache.py
83-83: No whitespaces allowed surrounding docstring text
(D210)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Check commit
- GitHub Check: Check commit
Summary by CodeRabbit
Bug Fixes
Style
Documentation
Tests
Noneand improved formatting in test data.Chores