Skip to content

Refactor BSBLANConnectionError and unify temperature range handling#1437

Merged
liudger merged 2 commits intomainfrom
refactor/cleanup-and-unify
Apr 11, 2026
Merged

Refactor BSBLANConnectionError and unify temperature range handling#1437
liudger merged 2 commits intomainfrom
refactor/cleanup-and-unify

Conversation

@liudger
Copy link
Copy Markdown
Owner

@liudger liudger commented Apr 11, 2026

This pull request makes significant refactoring and cleanup changes to the BSBLAN codebase, primarily focused on removing legacy temperature range handling, simplifying API initialization, and improving test reliability. The changes modernize how temperature ranges are managed, eliminate deprecated or redundant methods, and update tests to match the new logic. Additionally, exception handling is streamlined for clarity.

Temperature range handling:

  • Removed legacy global temperature range fields (_min_temp, _max_temp, _temperature_range_initialized) in favor of a unified per-circuit storage approach using _circuit_temp_ranges and _circuit_temp_initialized. All logic and tests now use this model for all circuits, including circuit 1. [1] [2] [3] [4] [5]

API initialization and validation:

  • Removed the deprecated async def _initialize_api_validator and async def _initialize_api_data methods, consolidating API initialization into _setup_api_validator and _copy_api_config. All references and tests are updated accordingly. [1] [2] [3] [4] [5] [6] [7] [8] [9]

Asynchronous API parameter extraction:

  • Refactored _extract_params_summary to be synchronous, updating all its call sites and related test mocks to remove unnecessary await statements. [1] [2] [3] [4] [5] [6]

Exception handling improvements:

  • Updated BSBLANConnectionError to accept a message parameter instead of a response, and streamlined its initialization logic.

Test cleanup and modernization:

  • Removed tests related to deprecated API initialization logic and updated remaining tests to use the new _copy_api_config and _setup_api_validator methods. This ensures tests are aligned with the refactored code and do not rely on legacy fields or methods. [1] [2] [3] [4] [5] [6] [7] [8]

These changes result in a cleaner, more maintainable codebase, with a consistent approach to temperature range management and API initialization.

liudger added 2 commits April 11, 2026 11:17
The BSBLANConnectionError.__init__ was calling super().__init__(self.message)
which referenced the attribute before it was set via the parent. Changed to
pass the message parameter directly via super().__init__(message) and removed
the unused self.response attribute.
…elpers

- Unify temperature range storage: replace legacy _min_temp/_max_temp with
  per-circuit _circuit_temp_ranges dict, enabling proper multi-circuit support
- Make _extract_params_summary synchronous (no I/O, was unnecessarily async)
- Remove deprecated _initialize_api_data method (unused since _copy_api_config)
- Remove deprecated _initialize_api_validator method (replaced by
  _setup_api_validator)
- Update all affected tests to match the refactored internals
Copilot AI review requested due to automatic review settings April 11, 2026 09:30
@liudger liudger added bugfix Inconsistencies or issues which will cause a problem for users or implementers. refactor Improvement of existing code, not introducing new features. labels Apr 11, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.89%. Comparing base (b3ee7c4) to head (cf92a56).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1437      +/-   ##
==========================================
- Coverage   99.89%   99.89%   -0.01%     
==========================================
  Files           6        6              
  Lines         988      955      -33     
  Branches      138      128      -10     
==========================================
- Hits          987      954      -33     
  Partials        1        1              

☔ 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.

@sonarqubecloud
Copy link
Copy Markdown

@liudger liudger merged commit c79da53 into main Apr 11, 2026
18 checks passed
@liudger liudger deleted the refactor/cleanup-and-unify branch April 11, 2026 09:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors python-bsblan internals to remove legacy initialization paths and unify temperature range handling across heating circuits, while updating the test suite to match the new lazy-loading/validation flow.

Changes:

  • Replaced legacy global temperature range fields with unified per-circuit storage (_circuit_temp_ranges / _circuit_temp_initialized) for all circuits.
  • Consolidated/remodeled API initialization by removing deprecated _initialize_api_* methods and relying on _setup_api_validator() + _copy_api_config().
  • Made _extract_params_summary() synchronous and updated call sites/tests accordingly; modernized and trimmed tests that targeted removed legacy paths.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/bsblan/bsblan.py Removes legacy API init + legacy HC1 temp-range fields; switches to unified per-circuit storage and sync param summarization.
src/bsblan/exceptions.py Refactors BSBLANConnectionError initialization to accept a message directly.
tests/conftest.py Removes mocking of deprecated _initialize_api_data; keeps request mocking.
tests/test_api_initialization.py Updates tests to validate _setup_api_validator() / _copy_api_config() behavior instead of removed legacy init.
tests/test_api_validation.py Updates mocks to match _extract_params_summary() now being synchronous.
tests/test_bsblan_edge_cases.py Removes edge-case tests tied to deprecated API init logic.
tests/test_circuit.py Updates circuit fixtures/assertions to use per-circuit temperature ranges (including HC1).
tests/test_initialization.py Replaces deprecated API init tests with _copy_api_config() / _setup_api_validator() tests.
tests/test_temperature_unit.py Updates temperature-range assertions to use per-circuit storage.
tests/test_temperature_validation.py Updates temperature-range setup/assertions to use per-circuit storage.
tests/test_thermostat.py Updates thermostat fixture to seed per-circuit temp ranges.
tests/test_version_errors.py Renames test to target _setup_api_validator() instead of removed _initialize_api_validator().

Comment on lines +29 to +36
def __init__(self, message: str | None = None) -> None:
"""Initialize a new instance of the BSBLANConnectionError class.

Args:
response: Optional response message.
message: Optional error message.

"""
self.response = response
super().__init__(self.message)
super().__init__(message)
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

Renaming the constructor parameter from response to message is a breaking public API change for callers that instantiate BSBLANConnectionError with response= (it’s exported in bsblan.__init__). Consider keeping backwards compatibility by accepting the old keyword as an alias (or using a deprecated response kwarg that maps to message).

Copilot uses AI. Check for mistakes.
Comment on lines 23 to +36
class BSBLANConnectionError(BSBLANError):
"""BSBLAN connection exception."""

message_timeout: str = "Timeout occurred while connecting to BSBLAN device."
message_error: str = "Error occurred while connecting to BSBLAN device."

def __init__(self, response: str | None = None) -> None:
def __init__(self, message: str | None = None) -> None:
"""Initialize a new instance of the BSBLANConnectionError class.

Args:
response: Optional response message.
message: Optional error message.

"""
self.response = response
super().__init__(self.message)
super().__init__(message)
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

BSBLANConnectionError() without an explicit message currently falls back to BSBLANError.message ("Unexpected response...") because BSBLANConnectionError doesn’t override message and __init__ just forwards None. That produces a misleading exception message for a connection error; consider defaulting to message_error (or setting a class-level message) when no message is provided.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Inconsistencies or issues which will cause a problem for users or implementers. refactor Improvement of existing code, not introducing new features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants