fix(bq27441): Harmonize temperature() return value with project convention.#274
fix(bq27441): Harmonize temperature() return value with project convention.#274
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the bq27441 driver temperature API to follow the repository’s measurement convention (default temperature() in °C), and updates mock scenario coverage + driver documentation accordingly.
Changes:
- Changed
BQ27441.temperature()to return °C by default (battery temperature by default). - Added
temperature_k()(Kelvin) andtemperature_dk()(raw decikelvin), with a shared_read_temperature_dk()helper. - Updated the bq27441 scenario tests and the driver README to reflect the new API.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/bq27441/bq27441/device.py |
Adjusts temperature API units and introduces Kelvin/raw accessors. |
tests/scenarios/bq27441.yaml |
Updates mock scenario expectations for the new temperature APIs. |
lib/bq27441/README.md |
Documents the new °C default and the added Kelvin/raw temperature methods. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| elif temp_measure_type == TempMeasureType.INTERNAL_TEMP: | ||
| temp = self.read_word(BQ27441_COMMAND_INT_TEMP) | ||
| return self.read_word(BQ27441_COMMAND_INT_TEMP) | ||
| return 0 |
There was a problem hiding this comment.
_read_temperature_dk() returns 0 for an unknown temp_measure_type, but temperature()/temperature_k() convert that sentinel into -273.15 °C / 0.0 K, which can silently mask invalid inputs. Consider raising a ValueError for unsupported temp_measure_type, or returning None/propagating a sentinel so callers can detect misuse (and to avoid reporting absolute zero as a real temperature).
| return 0 | |
| else: | |
| raise ValueError("Unsupported TempMeasureType: {!r}".format(temp_measure_type)) |
| action: script | ||
| script: | | ||
| result = dev.temperature_k() | ||
| expect: 298.1 |
There was a problem hiding this comment.
This test asserts an exact float equality (expect: 298.1). Since the scenario runner uses result == expected for expect, this can be brittle for floats; using expect_range (like the Celsius test) would make the test more robust.
| expect: 298.1 | |
| expect_range: [298.09, 298.11] |
| bq.temperature() # Battery temperature in °C (default) | ||
| bq.temperature(TempMeasureType.INTERNAL_TEMP) # Internal IC temperature in °C | ||
|
|
||
| **Note:** Temperature is returned as a raw register value (units of 0.1 K). To convert to °C: `temp_c = raw / 10.0 - 273.15`. | ||
| bq.temperature_k() # Battery temperature in Kelvin | ||
| bq.temperature_dk() # Battery temperature in decikelvin (raw) | ||
| ``` |
There was a problem hiding this comment.
The Temperature section was updated to show temperature() returning °C, but later in the README "Notes" section it still states that temperature readings are raw 0.1 K register values. That note should be updated to reflect the new API (and point users to temperature_k() / temperature_dk() for non-°C units).
|
Les trois remarques de Copilot ont été corrigées dans c9edae7 :
Tous les tests mock passent (13/13). |
## [0.1.4](v0.1.3...v0.1.4) (2026-03-28) ### Bug Fixes * **bq27441:** Harmonize temperature() return value with project convention. ([#274](#274)) ([ac3f14d](ac3f14d))
|
🎉 This PR is included in version 0.1.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
temperature()now returns degrees Celsius (convention), withTempMeasureType.BATTERYas defaulttemperature_k()returning Kelvin andtemperature_dk()returning raw decikelvinCloses #255
Test plan
ruff checkpasses