-
Notifications
You must be signed in to change notification settings - Fork 8
Add try-except constructs for all set-functions #574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Note Reviews pausedUse the following commands to manage reviews:
Warning Rate limit exceeded@bouwew has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 59 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent updates primarily focus on enhancing exception handling across various modules, particularly within the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@coderabbitai review |
Actions performedReview triggered.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #574 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 3219 3309 +90
=========================================
+ Hits 3219 3309 +90 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- plugwise/init.py (3 hunks)
- plugwise/smile.py (11 hunks)
- tests/test_adam.py (1 hunks)
- tests/test_init.py (1 hunks)
Additional comments not posted (14)
tests/test_adam.py (1)
85-85
: Exception handling updated to catchConnectionFailedError
.Verification successful
The verification confirms that
ConnectionFailedError
is consistently used across the codebase for exception handling.
tests/test_init.py
: Multiple instances ofexcept pw_exceptions.ConnectionFailedError
tests/test_adam.py
: The updated instance ofexcept pw_exceptions.ConnectionFailedError
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `ConnectionFailedError` is consistently used across the codebase for similar operations. # Test: Search for exception handling patterns. Expect: Consistent use of `ConnectionFailedError`. rg --type python $'except pw_exceptions.ConnectionFailedError'Length of output: 94
Script:
#!/bin/bash # Description: Verify that `ConnectionFailedError` is consistently used across the codebase for similar operations. # Test: Search for exception handling patterns. Expect: Consistent use of `ConnectionFailedError`. rg 'except pw_exceptions.ConnectionFailedError' --glob '*.py'Length of output: 722
plugwise/__init__.py (11)
339-342
: Exception handling added toset_schedule_state
. Error handling is consistent with the PR objectives.
347-350
: Exception handling added toset_preset
. This enhances robustness by handling potential connection issues.
354-357
: Exception handling added toset_temperature
. This change improves error management during temperature setting operations.
366-369
: Exception handling added toset_number
. This is crucial for handling errors during critical operations like setting boiler temperatures.
373-376
: Exception handling added toset_temperature_offset
. This ensures robust error handling for thermostat offset adjustments.
382-385
: Exception handling added toset_switch_state
. This change is aligned with the PR's goal to enhance robustness by managing potential connection issues effectively.
389-392
: Exception handling added toset_gateway_mode
. This is important for managing gateway mode changes robustly.
396-399
: Exception handling added toset_regulation_mode
. This ensures that errors during regulation mode changes are handled properly.
403-406
: Exception handling added toset_dhw_mode
. This is crucial for handling errors during domestic hot water mode changes.
413-413
: Exception handling added todelete_notification
. This ensures that notification deletion errors are managed effectively.
420-420
: Exception handling added toreboot_gateway
. This is important for robustly managing potential issues during gateway reboots.plugwise/smile.py (1)
457-464
: Exception handling added tocall_request
. This centralizes error handling for network requests, aligning with the PR's objectives.Verification successful
Exception handling for
ConnectionFailedError
is consistently used across the codebase.
plugwise/smile.py
plugwise/__init__.py
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `ConnectionFailedError` is consistently used across the codebase for similar operations. # Test: Search for exception handling patterns. Expect: Consistent use of `ConnectionFailedError`. rg --type python $'except ConnectionFailedError'Length of output: 80
Script:
#!/bin/bash # Description: Verify that `ConnectionFailedError` is consistently used across the codebase for similar operations. # Test: Search for exception handling patterns. Expect: Consistent use of `ConnectionFailedError`. rg --type py $'except ConnectionFailedError'Length of output: 900
tests/test_init.py (1)
647-647
: The update to catchConnectionFailedError
specifically intinker_reboot
aligns well with the PR's objective to enhance specific exception handling.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- plugwise/init.py (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- plugwise/init.py
use to skip unnecessary testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Outside diff range and nitpick comments (1)
tests/test_init.py (1)
Line range hint
666-676
: Refactor exception handling intinker_switch
.The exception handling can be streamlined by combining the two
except
blocks forpw_exceptions.PlugwiseError
andpw_exceptions.ConnectionFailedError
into a single block using tuple syntax. This reduces redundancy and improves readability.- except pw_exceptions.PlugwiseError: - _LOGGER.info(" + locked, not switched as expected") - return False - except pw_exceptions.ConnectionFailedError: # leave for-loop at connect-error - if unhappy: - return True # test is pass! - _LOGGER.info(" + failed as expected") - else: # pragma: no cover - _LOGGER.info(" - failed unexpectedly") - return False + except (pw_exceptions.PlugwiseError, pw_exceptions.ConnectionFailedError) as e: + if isinstance(e, pw_exceptions.ConnectionFailedError) and unhappy: + _LOGGER.info(" + failed as expected") + return True + elif isinstance(e, pw_exceptions.PlugwiseError): + _LOGGER.info(" + locked, not switched as expected") + return False + else: + _LOGGER.info(" - failed unexpectedly") + return FalseAlso applies to: 678-678
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- plugwise/init.py (3 hunks)
- plugwise/legacy/smile.py (7 hunks)
- tests/test_adam.py (3 hunks)
- tests/test_anna.py (1 hunks)
- tests/test_init.py (17 hunks)
Files skipped from review as they are similar to previous changes (2)
- plugwise/init.py
- tests/test_adam.py
Additional comments not posted (5)
plugwise/legacy/smile.py (4)
8-8
: Added import forAny
fromtyping
.This is a standard practice for type hinting in Python, enhancing code readability and maintainability.
27-27
: Updated import to includeConnectionFailedError
.This change aligns with the PR's objective to enhance exception handling across the codebase.
293-300
: Introducedcall_request
method to centralize request handling with exception management.This method encapsulates the
_request
call within atry-except
block forConnectionFailedError
, centralizing error handling and reducing code duplication.
184-184
: Usage ofcall_request
in various set-functions.The use of
call_request
in these functions is consistent with the new error handling strategy, ensuring that all request-related errors are managed uniformly.Also applies to: 230-230, 258-258, 271-271, 291-291
tests/test_anna.py (1)
461-461
: Added calls totinker_dhw_mode
with and without theunhappy
parameter.These additions test the new
unhappy
parameter functionality, ensuring that the method behaves as expected under different conditions.Also applies to: 470-470
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
tests/test_legacy_anna.py (1)
42-42
: Add a comment explaining the purpose ofskip_testing
.It would be beneficial for maintainability to include a brief comment explaining when and why
skip_testing
should be set toTrue
. This helps other developers understand the intent behind this optional parameter.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- tests/test_adam.py (5 hunks)
- tests/test_anna.py (2 hunks)
- tests/test_legacy_anna.py (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- tests/test_adam.py
- tests/test_anna.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests/test_adam.py (5 hunks)
- tests/test_anna.py (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- tests/test_adam.py
- tests/test_anna.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- pyproject.toml (1 hunks)
Files skipped from review due to trivial changes (1)
- pyproject.toml
Additional context used
LanguageTool
CHANGELOG.md
[uncategorized] ~145-~145: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... Adam: remove use of control_state, xml-key no longer present. - Fix error in manua...
[uncategorized] ~174-~174: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...e typing: fix all type-ignores. - Clean up no longer used code. - Remove support f...
[uncategorized] ~245-~245: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...a/issues/347) - Change message request queue a FiFO queue ## v0.27.7: Stick bugfix:...
[style] ~291-~291: Consider using just “revert”. (RETURN_BACK)
Context: .../issues/81672) - _full_update_device(): revert back to v0.21.4 logic - async_update(): not ...
[style] ~295-~295: Consider an alternative to strengthen your wording. (IMPROVEMENTS_REFINEMENTS)
Context: ... Core #81712 ## v0.25.10: Thermostats: more improvements - Anna + Elga: hide cooling_enable swi...
[style] ~362-~362: Consider using just “reverted”. (RETURN_BACK)
Context: ...g a heating and a cooling setpoint, was reverted back to providing a single setpoint. ## v0....
[style] ~403-~403: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ...rovide the collected schedules as input in order to find the last-modified valid schedule. ...
[uncategorized] ~404-~404: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ied valid schedule. -_rule_ids_by_x()
: replace None by NONE, allowing for simp...
[uncategorized] ~405-~405: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...schedule_temperature
from output: for Adam the schedule temperature cannot be coll...
[grammar] ~452-~452: An apostrophe ‘s’ denotes possession. Did you mean to use the plural form of the noun (no apostrophe)? (NOUN_APOSTROPHE_S_VERB)
Context: ...dule_state() to handle multi thermostat scenario's - Improve tracking of the last used sch...
[style] ~478-~478: Consider an alternative to strengthen your wording. (CHANGES_ADJUSTMENTS)
Context: ...for #158 ## v0.16.7 - Smile: Bugfixes, more changes and improvements - Fix for #158: error...
[duplication] ~480-~480: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...ror setting up for systems with an Anna and and Elga (heatpump). - Block connecting to ...
[style] ~517-~517: ‘off of’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM_OFF_OF)
Context: ...upport (representation and switching on/off of a schedule has changed) - Anna: Fix coo...
[uncategorized] ~625-~625: Do not mix variants of the same word (‘initialisation’ and ‘initialization’) within a single text. (EN_WORD_COHERENCY)
Context: ...hat is needed: full_update_device() for initialisation, update-device() for updating of live d...
[uncategorized] ~633-~633: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2)
Context: ... state - Improvement: Prioritize request so requests like switching a relay get sen...
[grammar] ~633-~633: Double-check that this verb is correct. Did you mean to use the past participle or the infinitive form here? (GET_VBN)
Context: ... so requests like switching a relay get send out before power measurement requests. ...
[uncategorized] ~654-~654: This expression is usually spelled with a hyphen. (BASED_HYPHEN)
Context: ...urement -name
(string) - Name of device based om hardware model and MAC address -...
[grammar] ~916-~916: The singular proper name ‘Version’ must be used with a third-person or a past tense verb. (HE_VERB_AGR)
Context: ....16 - Fix central components - Version skip to align with-beta
-component ### 0....
[grammar] ~942-~942: The singular proper name ‘Version’ must be used with a third-person or a past tense verb. (HE_VERB_AGR)
Context: ...s - Tests updated accordingly - Version skip to align with-beta
-component ### 0....
[typographical] ~985-~985: Unpaired symbol: ‘'’ seems to be missing (EN_UNPAIRED_QUOTES)
Context: ...unctionality - Add individual Smiles as 'hub'-components ### 0.0.26 - Add relay ...
Additional comments not posted (2)
CHANGELOG.md (2)
5-5
: LGTM! The addition of missing exception handling for set-functions aligns with the PR objectives.
7-7
: LGTM! Updates to test code and code improvements are crucial for maintaining software quality.
@coderabbitai ignore |
@coderabbitai please approve, we'll check up on your recommendations at a later stage, outside of this PR |
@CoMPaTech, I've approved the PR as requested. Feel free to revisit the recommendations at your convenience later. If you need further assistance or have additional requests, just let me know! Actions performedComments resolved and changes approved. |
|
@coderabbitai ignore Actions performedReviews paused. |
Summary by CodeRabbit
New Features
call_request()
function for common exception handling.Bug Fixes
ConnectionFailedError
exceptions.Chores
0.38.1a0
inpyproject.toml
.CHANGELOG.md
.Tests