Skip to content

Conversation

@bouwew
Copy link
Contributor

@bouwew bouwew commented Sep 27, 2025

This update prohibits an unintentional nodetype.cache update that replaces the existing file with an empty file.

Summary by CodeRabbit

  • Bug Fixes

    • Prevents startup errors when the cache file is missing by returning an empty cache instead of failing.
  • Refactor

    • Defers cache-existence checks to read time for more predictable initialization.
    • Updates node-type update behavior and reduces noisy logs for invalid node types.
  • Chores

    • Bumps package version and updates changelog.

@coderabbitai
Copy link

coderabbitai bot commented Sep 27, 2025

Walkthrough

Deferred cache-file existence tracking from initialization to read time in plugwise_usb/helpers/cache.py. Renamed NetworkRegistrationCache.update_nodetypesupdate_nodetype and updated its call site in plugwise_usb/network/registry.py; the new guard also skips NodeType.CIRCLE_PLUS. Version bump and changelog entry added.

Changes

Cohort / File(s) Summary of Changes
Helper cache read flow
plugwise_usb/helpers/cache.py
Removed early evaluation of cache-file existence from initialization; read_cache now sets self._cache_file_exists = await ospath.exists(self._cache_file) and returns empty cache if the file does not exist.
Network cache API rename & guard
plugwise_usb/network/cache.py
Renamed method update_nodetypesupdate_nodetype; updated early-exit guard to return when node_type is None or NodeType.CIRCLE_PLUS (skips storing/updating these types); changed one log emission in restore_cache from warning to info with adjusted formatting.
Registry callsite update
plugwise_usb/network/registry.py
Updated call from NetworkRegistrationCache.update_nodetypes to NetworkRegistrationCache.update_nodetype.
Tests
tests/test_usb.py
Updated test to call update_nodetype (renamed API) with same arguments.
Metadata
CHANGELOG.md, pyproject.toml
Added changelog entry for v0.47.1 (2025-09-27, PR 351) and bumped project version from 0.47.00.47.1; minor changelog formatting tweak.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant CacheHelper as Helper (plugwise_usb/helpers/cache.py)
  participant FS as FileSystem

  Caller->>CacheHelper: initialize_cache()
  Note right of CacheHelper: no existence flag set at init
  Caller->>CacheHelper: read_cache()
  CacheHelper->>FS: exists(self._cache_file)?
  alt file exists
    CacheHelper->>FS: read file
    FS-->>CacheHelper: data
    CacheHelper-->>Caller: parsed cache
  else file missing
    CacheHelper-->>Caller: empty cache
  end
Loading
sequenceDiagram
  autonumber
  actor RegistryCaller
  participant Registry as NetworkRegistry
  participant NetCache as NetworkRegistrationCache

  RegistryCaller->>Registry: update_network_nodetype(mac, node_type)
  Registry->>NetCache: update_nodetype(mac, node_type)
  alt node_type is None or CIRCLE_PLUS
    NetCache-->>Registry: no-op (skipped)
  else
    NetCache->>NetCache: validate/update nodetypes and save
    NetCache-->>Registry: ack
  end
  Registry-->>RegistryCaller: done
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • dirixmjm

Poem

In my burrow I nibble bytes and hum,
I move a flag and rename with a drum.
I skip the plus that hops too high,
If the cache is gone, I watch the sky. 🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title “Continuous cache improving” is vague and does not clearly summarize the primary change of preventing unintended empty cache file writes by deferring the cache existence check; it also feels grammatically awkward and fails to convey the specific update to the cache logic. Please revise the title to accurately reflect the main change, for example “Prevent unintended empty cache file creation by deferring existence checks” or “Defer cache file existence check to read time to avoid empty writes.”
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch improve-cache-3

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d9fcaa and ce5f18e.

📒 Files selected for processing (1)
  • pyproject.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Process test coverage

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Sep 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.08%. Comparing base (b757359) to head (ce5f18e).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #351      +/-   ##
==========================================
- Coverage   82.14%   82.08%   -0.07%     
==========================================
  Files          36       36              
  Lines        8171     8171              
==========================================
- Hits         6712     6707       -5     
- Misses       1459     1464       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@coderabbitai coderabbitai bot left a 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

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b757359 and 80721f6.

📒 Files selected for processing (2)
  • plugwise_usb/helpers/cache.py (1 hunks)
  • plugwise_usb/network/cache.py (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#329
File: plugwise_usb/nodes/circle.py:0-0
Timestamp: 2025-08-30T10:46:42.649Z
Learning: In plugwise_usb/nodes/circle.py, the save_cache() method should only be called when both cache_updated AND save_cache parameters are True. If no cache updates occurred (cache_updated = False), there's no meaningful data to persist to disk, even if the caller requested saving (save_cache = True). This prevents unnecessary disk I/O operations.
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#255
File: plugwise_usb/nodes/circle.py:615-627
Timestamp: 2025-06-20T07:58:41.534Z
Learning: In the plugwise_usb/nodes/circle.py file, the `log_addresses_missing` property being `None` means the parameter has not been initialized yet, not that all logs are present. When `log_addresses_missing` is `None`, returning `False` from cache loading methods is correct because the system is not properly initialized and further execution would be useless.
📚 Learning: 2025-08-30T10:46:42.649Z
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#329
File: plugwise_usb/nodes/circle.py:0-0
Timestamp: 2025-08-30T10:46:42.649Z
Learning: In plugwise_usb/nodes/circle.py, the save_cache() method should only be called when both cache_updated AND save_cache parameters are True. If no cache updates occurred (cache_updated = False), there's no meaningful data to persist to disk, even if the caller requested saving (save_cache = True). This prevents unnecessary disk I/O operations.

Applied to files:

  • plugwise_usb/network/cache.py
🧬 Code graph analysis (1)
plugwise_usb/helpers/cache.py (1)
tests/test_usb.py (1)
  • exists (209-221)

@bouwew bouwew marked this pull request as ready for review September 27, 2025 12:37
@bouwew bouwew requested a review from a team as a code owner September 27, 2025 12:37
Copy link
Contributor

@dirixmjm dirixmjm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sonarqubecloud
Copy link

@bouwew bouwew merged commit dec9a83 into main Sep 27, 2025
16 of 17 checks passed
@bouwew bouwew deleted the improve-cache-3 branch September 27, 2025 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants