Catch up #170
Catch up #170
Conversation
CoMPaTech
added
enhancement
WalkthroughThe recent changes enhance the project's configuration and code quality by updating GitHub Actions workflows, modifying installation scripts, and refining docstrings. Significant updates include incrementing caching versions, introducing a new installation prefix ( Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CI_Workflow
participant Package_Manager
Developer->>CI_Workflow: Trigger CI Process
CI_Workflow->>Package_Manager: Install Dependencies (with `uv`)
Package_Manager->>CI_Workflow: Dependencies Installed
CI_Workflow->>Developer: CI Process Completed
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 Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Outside diff range, codebase verification and nitpick comments (12)
CHANGELOG.md (1)
3-3: Ensure consistent section titles.The new section titled "Ongoing" should have a consistent format with other sections, such as including a version number or date if applicable.
plugwise_usb/nodes/circle.py (11)
258-258: Add a period at the end of the docstring.The docstring should end with a period for consistency.
- """Process received message""" + """Process received message."""Tools
Ruff
258-258: First line should end with a period
Add period
(D400)
258-258: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
392-392: Refine the docstring for clarity and consistency.The docstring should be in the imperative mood and end with a period.
- """Converts the amount of pulses to kWs using the calaboration offsets""" + """Convert the amount of pulses to kWs using the calibration offsets."""Tools
Ruff
392-392: First line should end with a period
Add period
(D400)
392-392: First line of docstring should be in imperative mood: "Converts the amount of pulses to kWs using the calaboration offsets"
(D401)
392-392: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
224-248: Ensure logging consistency and clarity.The logging statements within the
request_power_updatemethod should be consistent and clear. Consider combining or rephrasing for better readability.- _LOGGER.info( - "Queue _last_log_address for %s at %s last_collected %s", - str(self.mac), - str(self._last_log_address), - self._energy_last_collected_timestamp, - ) + _LOGGER.info( + "Queue _last_log_address for %s at %s last_collected %s", + self.mac, + self._last_log_address, + self._energy_last_collected_timestamp, + )
233-248: Clarify conditional block for failed address handling.The conditional block handling failed addresses can be made clearer by adding comments or refactoring.
- if len(self._energy_history_failed_address) > 0: - _mem_address = self._energy_history_failed_address.pop(0) - if self._energy_memory.get(_mem_address, 0) < 4: - _LOGGER.info( - "Collect EnergyCounters for %s at %s", - str(self.mac), - str(_mem_address), - ) - self.request_energy_counters(_mem_address) - self._energy_ratelimit_collection_timestamp = _timestamp_utcnow - else: - _LOGGER.info( - "Drop known request_energy_counters for %s at %s and clock sync", - str(self.mac), - str(_mem_address), - ) - self.get_clock(self.sync_clock) + if self._energy_history_failed_address: + _mem_address = self._energy_history_failed_address.pop(0) + if self._energy_memory.get(_mem_address, 0) < 4: + _LOGGER.info( + "Collect EnergyCounters for %s at %s", + self.mac, + _mem_address, + ) + self.request_energy_counters(_mem_address) + self._energy_ratelimit_collection_timestamp = _timestamp_utcnow + else: + _LOGGER.info( + "Drop known request_energy_counters for %s at %s and clock sync", + self.mac, + _mem_address, + ) + self.get_clock(self.sync_clock)
Line range hint
502-527:
Refactor complex conditionals for readability.The complex conditionals within the
_update_energy_today_nowmethod can be refactored for better readability.- self._energy_pulses_today_now is not None - and self._energy_pulses_today_now > _pulses_today_now - and self._energy_pulses_midnight_rollover < _local_midnight_timestamp + self._energy_pulses_today_now is not None and + self._energy_pulses_today_now > _pulses_today_now and + self._energy_pulses_midnight_rollover < _local_midnight_timestamp
624-631: Improve logging message clarity.The logging message in
request_energy_countersshould be more descriptive.- _LOGGER.debug( - "Skip request_energy_counters for % is unavailable", - str(self.mac), - ) + _LOGGER.debug( + "Skipping request_energy_counters for %s as it is unavailable", + self.mac, + )
696-701: Remove redundantstr()calls in logging.The
str()calls in the logging statements are redundant and can be removed for cleaner code.- _LOGGER.info( - "push _energy_memory for %s address %s slot %s stamp %s", - str(self.mac), - str(message.logaddr.value), - str(_slot), - str(_log_timestamp), - ) + _LOGGER.info( + "push _energy_memory for %s address %s slot %s stamp %s", + self.mac, + message.logaddr.value, + _slot, + _log_timestamp, + )
720-724: Remove redundantstr()calls in logging.The
str()calls in the logging statements are redundant and can be removed for cleaner code.- _LOGGER.info( - "history_rollover %s %s %s", - str(_log_timestamp), - str(_utc_hour_timestamp), - str(self._energy_last_rollover_timestamp), - ) + _LOGGER.info( + "history_rollover %s %s %s", + _log_timestamp, + _utc_hour_timestamp, + self._energy_last_rollover_timestamp, + )
763-768: Remove redundantstr()calls in logging.The
str()calls in the logging statements are redundant and can be removed for cleaner code.- _LOGGER.info( - "CircleEnergyCounter failed for %s at %s|%s count %s", - str(self.mac), - str(message.logaddr.value), - str(self._last_log_address), - str(self._energy_last_collected_count), - ) + _LOGGER.info( + "CircleEnergyCounter failed for %s at %s|%s count %s", + self.mac, + message.logaddr.value, + self._last_log_address, + self._energy_last_collected_count, + )
773-784: Clarify conditional block for failed address handling.The conditional block handling failed addresses can be made clearer by adding comments or refactoring.
- if ( - message.logaddr.value == self._last_log_address - and self._energy_last_collected_count > 3 - ): - if ( - self._energy_history_failed_address.count( - self._last_log_address - 1 - ) - == 0 - ): - self._energy_history_failed_address.append( - self._last_log_address - 1 - ) + if message.logaddr.value == self._last_log_address and self._energy_last_collected_count > 3: + if self._energy_history_failed_address.count(self._last_log_address - 1) == 0: + self._energy_history_failed_address.append(self._last_log_address - 1)
Line range hint
850-880:
Remove redundantstr()calls in logging.The
str()calls in the logging statements are redundant and can be removed for cleaner code.- _LOGGER.info( - "Calculated address %s at %s from %s at %s with %s|%s", - str(_address), - str(utc_timestamp), - str(self._last_log_address), - str(_utc_now_timestamp), - str(_sslot), - str(_hours_offset), - ) + _LOGGER.info( + "Calculated address %s at %s from %s at %s with %s|%s", + _address, + utc_timestamp, + self._last_log_address, + _utc_now_timestamp, + _sslot, + _hours_offset, + )
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- .github/workflows/merge.yml (2 hunks)
- .github/workflows/verify.yml (16 hunks)
- CHANGELOG.md (1 hunks)
- plugwise_usb/init.py (1 hunks)
- plugwise_usb/controller.py (1 hunks)
- plugwise_usb/messages/responses.py (2 hunks)
- plugwise_usb/nodes/circle.py (16 hunks)
- plugwise_usb/nodes/circle_plus.py (1 hunks)
- plugwise_usb/nodes/scan.py (1 hunks)
- plugwise_usb/nodes/sed.py (1 hunks)
- plugwise_usb/nodes/sense.py (1 hunks)
- plugwise_usb/nodes/switch.py (1 hunks)
- plugwise_usb/parser.py (2 hunks)
- pyproject.toml (8 hunks)
- scripts/setup.sh (1 hunks)
- scripts/setup_test.sh (1 hunks)
- scripts/tests_and_coverage.sh (1 hunks)
Files skipped from review due to trivial changes (3)
- plugwise_usb/controller.py
- plugwise_usb/messages/responses.py
- scripts/tests_and_coverage.sh
Additional context used
Ruff
plugwise_usb/nodes/switch.py
30-30: First line should end with a period
Add period
(D400)
30-30: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
plugwise_usb/nodes/sense.py
47-47: First line should end with a period
Add period
(D400)
47-47: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
plugwise_usb/nodes/scan.py
48-48: First line should end with a period
Add period
(D400)
48-48: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
plugwise_usb/parser.py
38-38: First line should end with a period
Add period
(D400)
38-38: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
46-46: First line should end with a period
Add period
(D400)
46-46: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
59-59: First line should end with a period
Add period
(D400)
59-59: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
plugwise_usb/nodes/sed.py
45-45: First line should end with a period
Add period
(D400)
45-45: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
plugwise_usb/nodes/circle_plus.py
29-29: First line should end with a period
Add period
(D400)
29-29: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
plugwise_usb/__init__.py
541-541: First line should end with a period
Add period
(D400)
541-541: First line of docstring should be in imperative mood: "Main worker loop to watch all other worker threads"
(D401)
541-541: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
plugwise_usb/nodes/circle.py
258-258: First line should end with a period
Add period
(D400)
258-258: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
392-392: First line should end with a period
Add period
(D400)
392-392: First line of docstring should be in imperative mood: "Converts the amount of pulses to kWs using the calaboration offsets"
(D401)
392-392: First line should end with a period, question mark, or exclamation point
Add closing punctuation
(D415)
486-486:
datetime.datetime.utcnow()used(DTZ003)
Additional comments not posted (22)
scripts/setup.sh (3)
14-14: Verify the necessity of theuvpackage.Ensure that the
uvpackage is required for the setup process and that it is correctly implemented.
15-15: Verify the necessity of theuvprefix forpip install wheel.Ensure that the
uvprefix is required for thepip install wheelcommand and that it is correctly implemented.
16-16: Verify the necessity of theuvprefix forpip install --upgrade.Ensure that the
uvprefix is required for thepip install --upgradecommand and that it is correctly implemented.scripts/setup_test.sh (2)
14-14: Verify the necessity of theuvpackage.Ensure that the
uvpackage is required for the setup process and that it is correctly implemented.
15-15: Verify the necessity of theuvprefix forpip install --upgrade.Ensure that the
uvprefix is required for thepip install --upgradecommand and that it is correctly implemented..github/workflows/merge.yml (2)
7-7: Verify the necessity of updatingCACHE_VERSIONto21.Ensure that the update to the
CACHE_VERSIONenvironment variable is required and correctly implemented.
25-25: Verify the necessity of updatingactions/checkouttov4.Ensure that the update to the
actions/checkoutaction version is required and correctly implemented.pyproject.toml (10)
26-26: Addition of new maintainer approved.The addition of "arnoutd_77" to the maintainers list looks good.
30-30: Addition of new maintainer approved.The addition of "dirixmjm" to the maintainers list looks good.
56-56: Updatetarget-versionforblackapproved.The update to use "py312" for
blackis approved.
224-224: Updateselectlist forruffapproved.The update to use
lint.selectforruffis approved.
291-291: Updateignorelist forruffapproved.The update to use
lint.ignoreforruffis approved.
315-315: Updateflake8-import-conventionssection approved.The update to use
lint.flake8-import-conventionsforruffis approved.
323-323: Updateflake8-pytest-stylesection approved.The update to use
lint.flake8-pytest-styleforruffis approved.
326-326: Updatemccabesection approved.The update to use
lint.mccabeforruffis approved.
192-192: Updatepython_versionformypyapproved.The update to use Python "3.12" for
mypyis approved.Ensure that the
mypyconfiguration is compatible with Python 3.12.Verification successful
Update
python_versionformypyapproved.The update to use Python "3.12" for
mypyis approved. The configuration does not contain any Python 3.12-specific features or incompatibilities.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify `mypy` configuration compatibility with Python 3.12. # Test: Check for any Python 3.12-specific features or incompatibilities in `mypy` configuration. rg --type python -A 5 $'mypy'Length of output: 59
Script:
#!/bin/bash # Description: Verify `mypy` configuration compatibility with Python 3.12. # Test: Check for any Python 3.12-specific features or incompatibilities in `mypy` configuration within the `pyproject.toml` file. rg -A 5 'mypy' pyproject.tomlLength of output: 168
32-32: Updaterequires-pythonversion approved.The update to require Python ">=3.10.0" is approved.
Ensure that all parts of the codebase are compatible with Python 3.10.
Verification successful
We need to verify the compatibility of the codebase with Python 3.10. Let's search for any version-specific features or deprecated features that might affect compatibility.
Update
requires-pythonversion approved.The update to require Python ">=3.10.0" is approved. No specific incompatibilities with Python 3.10 were found in the codebase.
- Ensure that all parts of the codebase are compatible with Python 3.10.
- No specific
sys.version_infochecks or deprecated features were detected that would affect compatibility.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility with Python 3.10. # Test: Check for any Python 3.10-specific features or incompatibilities. rg --type python -A 5 $'python_version'Length of output: 69
Script:
#!/bin/bash # Description: Verify compatibility with Python 3.10. # Test: Check for any Python 3.10-specific features or incompatibilities by searching for version checks and deprecated features. # Search for version checks in the codebase. rg 'sys\.version_info' -A 5 # Search for deprecated features that might affect compatibility with Python 3.10. rg 'import \w+' -A 5Length of output: 15658
.github/workflows/verify.yml (5)
7-7: UpdateCACHE_VERSIONapproved.The update to
CACHE_VERSION22 is approved.
25-25: Updateactions/checkoutversion approved.The update to use
actions/checkout@v4is approved.Also applies to: 75-75, 128-128, 179-179, 220-220, 256-256, 296-296, 306-306, 316-316, 361-361, 404-404
101-101: Updateruffcommand toruff checkapproved.The update to use
ruff checkis approved.Also applies to: 111-111
111-111: Updateruff --fixcommand toruff check --fixapproved.The update to use
ruff check --fixis approved.
51-53: Update installation commands to useuvapproved.The update to use
uvfor installation commands is approved.Ensure that the
uvtool or wrapper is correctly applied and does not cause any issues.Also applies to: 203-208
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/verify.yml (16 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/verify.yml
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #170 +/- ##
==========================================
- Coverage 35.17% 35.14% -0.03%
==========================================
Files 21 21
Lines 2405 2404 -1
==========================================
- Hits 846 845 -1
Misses 1559 1559 ☔ View full report in Codecov by Sentry. |
|
@coderabbitai please review again in full, that is, including updating all outdated conversations, they seem legit enough though can't be committed in batch due to the reference being outdated. Thanks in advance |
|
@CoMPaTech, I have reviewed the changes in full. Here is a detailed summary of the modifications: .github/workflows/merge.yml
.github/workflows/verify.yml
CHANGELOG.md
plugwise_usb/init.py
plugwise_usb/controller.py
plugwise_usb/messages/responses.py
plugwise_usb/nodes/circle.py
plugwise_usb/nodes/circle_plus.py
plugwise_usb/nodes/scan.py
plugwise_usb/nodes/sed.py
plugwise_usb/nodes/sense.py
plugwise_usb/nodes/switch.py
plugwise_usb/parser.py
pyproject.toml
scripts/setup.sh
scripts/setup_test.sh
scripts/tests_and_coverage.sh
I have also ensured that all previous conversations are addressed and updated as necessary. Analysis chainScripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Fetch the diff of the entire PR to analyze all changes.
git diff origin/main...HEAD
Length of output: 32370 Actions performedReview triggered.
|
|
OK, most of the nitpicks by CodeRabbit are ruff anyway, so basically updating each and every doctstring properly. (Which is also why I left ruff out for now, will add an issue to ensure someone picks this up). As for the red crosses, nothing to see there:
If this is accepted/merged please tick the LOWEST checkbox on #3 ( |
|
@coderabbitai please approve, your |
|
|
Also replaced prettier with biome (see also plugwise/plugwise-beta#695) and missing changelog entry as well as other pre-commit updates. This should supersede #157 and close it as incomplete. |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores
uv, in setup scripts.