Lower connection-timeout for actual devices after initial connect #576
Conversation
WalkthroughThe recent updates primarily address timeout configurations across 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #576 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 3309 3316 +7
=========================================
+ Hits 3309 3316 +7 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- plugwise/init.py (6 hunks)
- plugwise/constants.py (1 hunks)
- plugwise/legacy/smile.py (4 hunks)
- plugwise/smile.py (4 hunks)
- tests/test_init.py (1 hunks)
Additional comments not posted (9)
plugwise/legacy/smile.py (2)
43-43: The addition of thetimeoutparameter to the constructor is well-implemented and aligns with the PR's objectives to manage timeouts effectively.
79-79: Setting the_timeoutattribute directly from thetimeoutparameter is clear and effective.plugwise/constants.py (2)
35-35: Reducing theDEFAULT_TIMEOUTfrom 30 to 10 is a sensible change that should improve responsiveness without compromising functionality.
36-36: IntroducingDEFAULT_LEGACY_TIMEOUTas a separate constant for legacy devices is a prudent decision to accommodate devices that may need longer connection times.plugwise/__init__.py (2)
50-50: Setting the default timeout in the constructor toDEFAULT_LEGACY_TIMEOUTaligns with the updated timeout strategy and ensures backward compatibility with legacy devices.
197-197: Dynamically setting_timeoutbased on device type in_smile_detectis an effective way to handle the varying needs of different devices.plugwise/smile.py (2)
Line range hint
49-88: The addition of thetimeoutparameter in the constructor is consistent with the PR's objective to manage timeouts more effectively. This change allows for dynamic timeout settings, which can be crucial for different network conditions or device responsiveness.
88-88: Correctly settingself._timeoutto the passedtimeoutparameter ensures that the timeout behavior is consistent across the class methods that rely on this setting.tests/test_init.py (1)
555-558: The added assertions forsmile._timeoutare crucial for ensuring that the timeout settings are correctly applied based on the device type (legacy or not). This is a good practice to confirm that the system behaves as expected under different configurations.
There was a problem hiding this comment.
Actionable comments posted: 0
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
[style] ~295-~295: Consider using just “revert”. (RETURN_BACK)
Context: .../issues/81672) - _full_update_device(): revert back to v0.21.4 logic - async_update(): not ...
[style] ~299-~299: 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] ~366-~366: Consider using just “reverted”. (RETURN_BACK)
Context: ...g a heating and a cooling setpoint, was reverted back to providing a single setpoint. ## v0....
[style] ~407-~407: 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] ~408-~408: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ied valid schedule. -_rule_ids_by_x(): replace None by NONE, allowing for simp...
[grammar] ~456-~456: 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] ~482-~482: 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] ~484-~484: 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] ~521-~521: ‘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] ~629-~629: 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] ~637-~637: 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] ~637-~637: 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] ~658-~658: 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] ~920-~920: 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] ~946-~946: 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] ~989-~989: 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 (1)
CHANGELOG.md (1)
3-6: The changelog entry for version 0.38.2 succinctly captures the essence of the PR, which is to lower the connection timeout for actual devices after the initial connect. This aligns well with the PR objectives and the modifications described in the AI-generated summary.
|
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
pyproject.tomlto0.38.2.Tests