Type P1 sensors as float #648
Conversation
WalkthroughThe changes in this pull request involve modifications to various JSON configuration files and Python scripts, primarily focusing on the representation of numeric values. Integer values have been updated to floating-point representations across multiple devices and settings, enhancing the precision of measurements. Additionally, specific sensor values and attributes have been adjusted, including updates to the illuminance sensor for a thermostat and changes to the data types of several attributes in the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@CoMPaTech is this the rounding we want? Always show one decimal at minimum? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #648 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 3370 3368 -2
=========================================
- Hits 3370 3368 -2 ☔ 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: 5
🧹 Outside diff range and nitpick comments (8)
tests/data/anna/legacy_anna_2.json (1)
Line range hint
1-62: Consider documenting data type expectationsSince this is a test fixture file that others might use as reference, consider adding a comment at the top of the file or in the test documentation that explicitly states which sensor types should be represented as float vs integer. This would help maintain consistency as new test cases are added.
tests/data/anna/anna_elga_2_cooling.json (1)
Line range hint
16-78: Consider documenting temperature precision requirements.While the conversion to floating-point values is consistent throughout the file, it would be helpful to document the required precision for different types of temperature measurements (e.g., setpoints vs. sensor readings) in the project documentation.
Would you like me to help create documentation that specifies the precision requirements for different temperature measurements?
tests/data/anna/anna_loria_driessens.json (2)
43-46: Consider using higher precision for temperature offset.While the conversion to float is correct, given that the resolution is 0.1 (line 44), consider using the same precision for all values in this section for consistency.
- "lower_bound": -2.0, + "lower_bound": -2.00, "resolution": 0.1, - "setpoint": 0.0, - "upper_bound": 2.0 + "setpoint": 0.00, + "upper_bound": 2.00
87-90: Consider standardizing sensor value precision.The sensor readings show inconsistent decimal places:
- intended_boiler_temperature: 0.0
- return_temperature: 23.0
This might affect data consistency when processing these values.- "intended_boiler_temperature": 0.0, + "intended_boiler_temperature": 0.00, "modulation_level": 0, "outdoor_air_temperature": 7.5, - "return_temperature": 23.0, + "return_temperature": 23.00,tests/data/adam/adam_plus_anna.json (1)
Found two integer values that need to be converted to floats
In the file
tests/data/adam/adam_heatpump_cooling.json, there are two values that should be converted to floats for consistency:
"setpoint": 18,"temperature": 22All other numeric values in the test data files appear to be properly represented as floats.
🔗 Analysis chain
Line range hint
1-103: Verify consistency across other test data filesWhile the changes in this file look good, let's verify that similar conversions have been applied consistently across other test data files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining integer values in similar test data files # that should have been converted to floats # Look for potential integer values in temperature and electricity fields rg -g '*.json' -A 1 '"(temperature|electricity|setpoint|bound)": \d+[^.]' tests/data/Length of output: 18145
tests/data/stretch/stretch_v31.json (1)
Found inconsistent number format in electricity values
There is an integer value
0instead of float0.0forelectricity_producedintests/data/stretch/stretch_v23.json. All other electricity-related values across test files are already in floating-point format.
tests/data/stretch/stretch_v23.json: Change"electricity_produced": 0to"electricity_produced": 0.0🔗 Analysis chain
Line range hint
1-143: Verify type consistency across related test filesThe changes look good and align with the PR objective. Let's verify that similar type conversions have been applied consistently across related test files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining integer values in electricity-related fields across test data files # Expected: All electricity values should be in floating-point format (e.g., 0.0, not 0) echo "Checking for potential inconsistent integer values in test data..." rg -g '*.json' '"electricity_(consumed|produced|consumed_interval)":\s*\d+[^.]' tests/data/Length of output: 2250
plugwise/util.py (1)
Line range hint
143-178: Consider updating function documentation.The function's behavior for different value ranges has changed significantly. Consider updating the docstring to document the new formatting rules:
- Values < 10: two decimal places
- Values >= 10: one decimal place
- Special units: specific formatting rules
tests/data/adam/adam_zone_per_device.json (1)
Line range hint
1-418: Overall consistent implementation with one suggestionThe conversion of integer values to floats is well-implemented across the configuration file, particularly for thermostat settings and temperature values. However, consider standardizing all electricity-related measurements (both consumed and produced) to use float representation for complete consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (35)
fixtures/legacy_anna/all_data.json(1 hunks)plugwise/constants.py(1 hunks)plugwise/util.py(1 hunks)tests/data/adam/adam_heatpump_cooling.json(21 hunks)tests/data/adam/adam_jip.json(14 hunks)tests/data/adam/adam_multiple_devices_per_zone.json(21 hunks)tests/data/adam/adam_plus_anna.json(5 hunks)tests/data/adam/adam_plus_anna_new_UPDATED_DATA.json(1 hunks)tests/data/adam/adam_zone_per_device.json(19 hunks)tests/data/anna/anna_elga_2.json(5 hunks)tests/data/anna/anna_elga_2_cooling.json(3 hunks)tests/data/anna/anna_elga_2_schedule_off.json(2 hunks)tests/data/anna/anna_elga_no_cooling.json(4 hunks)tests/data/anna/anna_heatpump_cooling.json(5 hunks)tests/data/anna/anna_heatpump_heating_UPDATED_DATA.json(2 hunks)tests/data/anna/anna_loria_cooling_active.json(4 hunks)tests/data/anna/anna_loria_driessens.json(2 hunks)tests/data/anna/anna_loria_heating_idle.json(4 hunks)tests/data/anna/anna_v4.json(3 hunks)tests/data/anna/anna_v4_UPDATED_DATA.json(3 hunks)tests/data/anna/anna_v4_dhw.json(3 hunks)tests/data/anna/anna_without_boiler_fw441.json(2 hunks)tests/data/anna/legacy_anna.json(3 hunks)tests/data/anna/legacy_anna_2.json(4 hunks)tests/data/p1/p1v4.json(1 hunks)tests/data/p1/p1v4_442_single.json(1 hunks)tests/data/p1/p1v4_442_single_UPDATED_DATA.json(1 hunks)tests/data/p1/p1v4_442_triple.json(1 hunks)tests/data/p1/smile_p1_v2.json(1 hunks)tests/data/p1/smile_p1_v2_2.json(1 hunks)tests/data/p1/smile_p1_v2_2_UPDATED_DATA.json(1 hunks)tests/data/stretch/stretch_v23.json(16 hunks)tests/data/stretch/stretch_v27_no_domain.json(1 hunks)tests/data/stretch/stretch_v31.json(5 hunks)tests/data/stretch/stretch_v31_UPDATED_DATA.json(3 hunks)
✅ Files skipped from review due to trivial changes (9)
- tests/data/adam/adam_heatpump_cooling.json
- tests/data/adam/adam_multiple_devices_per_zone.json
- tests/data/anna/anna_elga_2_schedule_off.json
- tests/data/anna/anna_heatpump_heating_UPDATED_DATA.json
- tests/data/anna/anna_v4_UPDATED_DATA.json
- tests/data/p1/p1v4_442_single_UPDATED_DATA.json
- tests/data/p1/smile_p1_v2_2_UPDATED_DATA.json
- tests/data/stretch/stretch_v27_no_domain.json
- tests/data/stretch/stretch_v31_UPDATED_DATA.json
🔇 Additional comments (82)
tests/data/p1/smile_p1_v2.json (4)
18-18: LGTM: Net electricity point conversion to float
The conversion of net_electricity_point to a floating-point value aligns with the PR objective.
19-19: LGTM: Electricity consumption values as floats
The conversion of consumption-related measurements (electricity_consumed_point, electricity_consumed_peak_interval, electricity_consumed_off_peak_interval) to floating-point values is consistent with the type standardization effort.
Also applies to: 23-24
25-25: LGTM: Electricity production values as floats
The conversion of production-related measurements (electricity_produced_point, electricity_produced_peak_interval, electricity_produced_off_peak_interval) to floating-point values maintains consistency with other sensor values.
Also applies to: 28-29
18-29: Verify data type consistency across the codebase
Let's ensure that the code handling these sensor values expects floating-point numbers.
✅ Verification successful
Based on the shell script results, I can provide a final response as the data shows clear type definitions in the codebase.
Floating-point values are correctly handled in the codebase
The codebase explicitly defines these sensor values as floating-point numbers in plugwise/constants.py:
net_electricity_point: floatelectricity_consumed_point: floatelectricity_produced_point: float
Additionally, all test fixtures consistently use decimal values (e.g., 458.0), confirming the expected floating-point representation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any potential integer type hints or assertions in the codebase
# that might conflict with the new floating-point values
# Search for type hints or assertions related to these sensor values
rg -l "net_electricity_point|electricity_consumed_point|electricity_produced_point" | xargs rg "int|float|assert"
# Search for any test cases that might need updating
fd -e py test_ | xargs rg "net_electricity_point|electricity_consumed_point|electricity_produced_point"
Length of output: 13977
tests/data/p1/smile_p1_v2_2.json (3)
18-19: LGTM! Consistent float conversion for electricity point values.
The conversion from integer to float format maintains precision while aligning with the PR objective.
23-24: LGTM! Consistent float conversion for interval measurements.
The conversion maintains precision while ensuring type consistency across all consumption measurements.
25-25: LGTM! Consistent float conversion for production metrics.
The conversion maintains consistency with other measurements. Note that cumulative values were already using float representation, suggesting this change completes the standardization of numeric types across all metrics.
Let's verify the consistency of float representation across other test files:
Also applies to: 28-29
✅ Verification successful
Float representation is consistently used across all electricity metrics
All numeric values in the P1 test files are properly represented with decimal points (X.0 for whole numbers and X.XXX for fractional values). This includes:
- Point values (e.g., "electricity_produced_point": 0.0)
- Cumulative values (e.g., "electricity_produced_peak_cumulative": 1296.136)
- Interval values (e.g., "electricity_produced_peak_interval": 0.0)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining integer values in P1 sensor data across test files
# Expected: All numeric values should be in float format (contain decimal point)
# Search for P1 test data files and check for integer values in sensor data
fd -e json -p 'tests/data/p1/' --exec rg -l '"sensors":\s*{[^}]*[^.0-9][0-9]+[^.0-9]'
Length of output: 1037
Script:
#!/bin/bash
# Let's try a simpler approach to find numeric values in P1 test files
# First, let's list all P1 test files and examine their content
# Find all P1 test files
fd -e json -p 'tests/data/p1/' --exec cat {} \; | \
# Extract lines containing numeric values in the sensors section
grep -A 5 '"sensors"' | \
# Show lines with numbers
grep -E '":.*[0-9]'
Length of output: 1783
Script:
#!/bin/bash
# Let's specifically check for electricity production values
# to verify consistency with the changes in the review
fd -e json -p 'tests/data/p1/' --exec cat {} \; | \
grep -E '"electricity_produced.*":.*[0-9]'
Length of output: 1993
tests/data/anna/anna_without_boiler_fw441.json (2)
27-29: LGTM! Consistent float representation for thermostat bounds
The conversion of integer values to floats for setpoint and bounds aligns with the 0.1 resolution setting and ensures consistent typing across the configuration.
39-39: LGTM! Consistent float representation for sensor setpoint
The setpoint conversion to float maintains consistency with other sensor values and the thermostat settings above.
tests/data/p1/p1v4.json (3)
24-26: LGTM! Consistent float conversion for electricity point readings.
The conversion to floating-point values maintains the correct precision and logical relationships between the readings.
30-37: LGTM! Consistent float conversion for interval and production readings.
The conversion maintains appropriate precision and the test values remain logically consistent with a consumption-only scenario.
24-37: Verify consistent float typing across the codebase.
Let's ensure this change is consistently applied across all P1-related files.
✅ Verification successful
Let me gather more specific information about the float values in these files.
Let me check how these values are handled in the Python code to ensure proper type handling.
Float typing is consistently implemented across the codebase
The verification shows that:
- All electricity-related fields in test JSON files consistently use float values (e.g.,
486.0,31610.031) - The type definitions in
plugwise/constants.pyexplicitly declare all electricity-related fields asfloattype - The test data changes align with the expected typing in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent float typing in P1-related files
# Test 1: Check other P1 test data files for integer values that should be float
echo "Checking P1 test data files for potential integer values..."
fd -e json -e py . tests/data/p1 --exec rg -l '[^\.]\d+[,}\s]'
# Test 2: Check related code for proper float typing
echo "Checking related code files for P1 sensor type definitions..."
rg -l "class.*SmileSensors|class.*P1" --type py
Length of output: 691
Script:
#!/bin/bash
# Check the actual content of these files focusing on electricity-related fields
echo "Checking p1v4_442_single.json..."
rg "electricity.*point|electricity.*cumulative|electricity.*interval" tests/data/p1/p1v4_442_single.json
echo -e "\nChecking p1v4_442_triple.json..."
rg "electricity.*point|electricity.*cumulative|electricity.*interval" tests/data/p1/p1v4_442_triple.json
echo -e "\nChecking smile_p1_v2.json..."
rg "electricity.*point|electricity.*cumulative|electricity.*interval" tests/data/p1/smile_p1_v2.json
# Check how these values are processed in the code
echo -e "\nChecking value processing in Python files..."
rg "electricity.*point|electricity.*cumulative|electricity.*interval" tests/test_p1.py tests/test_legacy_p1.py
Length of output: 2788
Script:
#!/bin/bash
# Check for float type handling in Python files
echo "Checking type handling in test files..."
ast-grep --pattern 'assert $_.$_ == $NUMBER' tests/test_p1.py tests/test_legacy_p1.py
echo -e "\nChecking property definitions..."
ast-grep --pattern 'property def $_($_):
$$$
return $_' tests/test_p1.py tests/test_legacy_p1.py
echo -e "\nChecking actual implementation..."
rg -A 2 "electricity.*point|electricity.*cumulative|electricity.*interval" plugwise/constants.py
Length of output: 3558
tests/data/p1/p1v4_442_single.json (1)
27-29: Verified: Consistent pattern with existing floating-point values.
The changes maintain consistency with the existing floating-point representation used for cumulative values (e.g., net_electricity_cumulative: 31610.031), ensuring a uniform approach to numeric representation across all sensor values.
tests/data/anna/legacy_anna.json (4)
21-22: LGTM! Consistent floating-point representation for thermostat bounds
The conversion to floating-point values aligns well with the thermostat's resolution of 0.1 and maintains consistency with other temperature values in the configuration.
41-44: LGTM! Consistent floating-point representation for boiler temperature settings
While the resolution is 1.0, using floating-point values maintains consistency with other temperature representations in the configuration.
53-53: LGTM! Consistent floating-point representation for intended boiler temperature
The conversion to float aligns with other temperature sensor values in the configuration.
30-30: LGTM! Appropriate floating-point representation for illuminance sensor
Converting illuminance to float provides better precision for light sensor readings.
✅ Verification successful
All illuminance values are consistently represented as floating-point numbers
The verification confirms that all illuminance values across the test data files are consistently using floating-point representation (e.g., 0.5, 86.0, 39.5, 150.8, etc.). This maintains data type consistency throughout the test suite.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify consistency of illuminance values across test data files
# Description: Check if all illuminance values are consistently represented as floats
# Search for illuminance values in all JSON test data files
fd -e json -e JSON -x rg -A 1 '"illuminance":'
Length of output: 1916
tests/data/anna/legacy_anna_2.json (3)
11-11: LGTM: Gateway temperature as float
The conversion of outdoor_temperature to float aligns with the PR objective of typing sensors as float.
23-25: LGTM: Thermostat values as float
The conversion of setpoint and boundary values to float is consistent with the temperature precision requirements. The resolution of 0.1 indicates that floating-point representation is appropriate here.
Also applies to: 36-36
45-48: Verify: Mixed data types in heater sensors
The changes correctly convert temperature-related values to float. However, I notice that modulation_level remains as an integer (line 58). This seems intentional as modulation levels are typically represented as percentages in whole numbers.
Let's verify if this mixed type approach is consistent across the codebase:
Also applies to: 55-59
✅ Verification successful
Mixed data types are consistently applied across test fixtures
The verification confirms that the data types are consistently used across all test fixtures:
modulation_levelis consistently represented as integers (0, 40, 52, 100) across all test files- Temperature-related fields (water, return, outdoor, dhw) are consistently using float values (e.g., 20.2, 46.3, 25.1)
This validates that the mixed type approach is intentional and correctly implemented throughout the test data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if modulation_level is consistently typed as integer across test fixtures
# and if other temperature values are consistently typed as float
# Search for modulation_level in test data files
echo "Checking modulation_level typing in test data:"
rg -g '*.json' '"modulation_level":\s*\d+[^.]' tests/
# Search for temperature-related fields to verify float typing
echo -e "\nChecking temperature value typing in test data:"
rg -g '*.json' '"(?:water|return|outdoor|dhw)_temperature":\s*\d+[^.]' tests/
Length of output: 5456
tests/data/p1/p1v4_442_triple.json (5)
24-26: LGTM: Basic power measurements correctly typed as float
The conversion of basic power measurements to float type is appropriate and maintains data precision.
30-31: LGTM: Interval measurements correctly typed as float
The interval measurements have been properly converted to float type, maintaining consistency with other power measurements.
32-37: LGTM: Production measurements correctly typed as float
The electricity production measurements have been appropriately converted to float type, even though they currently show zero values.
38-43: LGTM: Phase-specific measurements correctly typed as float
The phase-specific consumption and production measurements have been properly converted to float type, maintaining consistency across all electricity-related measurements.
24-43: Verify test coverage for float type handling
Since this is a test data file, we should ensure that the code handling these values properly accounts for the float type conversion.
fixtures/legacy_anna/all_data.json (1)
54-54: LGTM! The conversion to float aligns with the PR objective.
The change from integer (151) to float (150.8) for the illuminance sensor maintains consistency with other sensor values in the fixture and supports the broader initiative of typing P1 sensors as float.
Let's verify the pattern of float conversions in other fixture files:
✅ Verification successful
Illuminance values are consistently using float format across all test fixtures
The verification confirms that illuminance values are consistently represented as floating-point numbers across all fixture files in both fixtures/ and tests/ directories. All 32 occurrences found use decimal notation (e.g., 150.8, 60.0, 0.25, etc.), which validates that the conversion aligns with the established pattern in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar integer-to-float conversions in other fixture files
# Expected: Find other occurrences of illuminance values to confirm consistent typing
# Search for illuminance values in fixture and test files
rg -g '*.json' '"illuminance":\s*\d+\.?\d*' fixtures/ tests/
Length of output: 2355
tests/data/anna/anna_v4_dhw.json (4)
10-12: LGTM: Float conversion maintains value precision
The conversion to float notation is consistent with the PR objective while preserving the original values. Note that while the resolution is 1, keeping float notation ensures type consistency across P1 sensors.
16-18: LGTM: Float conversion matches resolution
The conversion to float notation is particularly appropriate here given the 0.01 resolution of the DHW temperature settings.
28-28: LGTM: Consistent float typing for all sensors
The conversion maintains consistency with other sensor values in the file that already use float notation.
Also applies to: 31-31
48-49: LGTM: Float bounds match thermostat resolution
The conversion to float notation is appropriate given the 0.1 resolution of the thermostat settings and maintains consistency with other temperature values.
tests/data/anna/anna_v4.json (2)
28-28: LGTM! Sensor values properly typed as float.
Temperature sensor values are correctly converted to float while maintaining appropriate types for other sensors (modulation_level as integer, water_pressure as float).
Also applies to: 31-31
48-49: LGTM! Thermostat bounds properly converted to float.
The conversion maintains the appropriate temperature range (4.0-30.0°C) while ensuring type consistency with the existing floating-point setpoint and resolution.
tests/data/anna/anna_heatpump_cooling.json (5)
15-15: LGTM: Consistent type conversion to float
The conversion of outdoor_temperature to float aligns with the PR objective.
25-28: LGTM: Consistent float conversion for boiler temperature settings
The conversion of temperature-related settings to float type is appropriate and maintains the original values.
42-43: LGTM: Consistent float conversion for sensor values
The conversion of temperature setpoints and measurements to float type is appropriate.
Also applies to: 47-47
62-65: LGTM: Consistent float conversion for thermostat settings
The conversion of temperature setpoints and bounds to float type is appropriate.
76-79: Verify: Duplicate setpoint values
The setpoint values appear in both the thermostat and sensors sections. While the float conversion is correct, please verify if this duplication is intentional for testing purposes.
✅ Verification successful
Duplicate setpoint values are intentional and by design
The verification shows that setpoint values are consistently duplicated between the thermostat and sensors sections across multiple test files. This pattern appears in all files where setpoints are defined, with matching values in both sections (e.g., 4/22, 19.5/30, 20.5/30, etc.). This duplication is intentional for the test data structure and represents the expected behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if this pattern of duplicate setpoints exists in other test files
echo "Checking for similar patterns in other test files..."
fd -e json -x grep -l "setpoint" {} \; | while read -r file; do
echo "=== $file ==="
jq 'recurse | select(has("thermostat") and has("sensors")) | select(.sensors.setpoint_low != null or .sensors.setpoint_high != null)' "$file"
done
Length of output: 25461
Script:
#!/bin/bash
# Let's try a simpler approach to check the structure of the specific file
echo "=== Checking anna_heatpump_cooling.json structure ==="
jq '.' tests/data/anna/anna_heatpump_cooling.json
echo -e "\n=== Checking if setpoint values appear in both thermostat and sensors sections ==="
jq '.thermostat.setpoint_low, .thermostat.setpoint_high, .sensors.setpoint_low, .sensors.setpoint_high' tests/data/anna/anna_heatpump_cooling.json
Length of output: 2813
Script:
#!/bin/bash
# Let's try accessing the nested structure correctly
echo "=== Checking setpoint values in the thermostat device ==="
jq '.["3cb70739631c4d17a86b8b12e8a5161b"].thermostat.setpoint_low,
.["3cb70739631c4d17a86b8b12e8a5161b"].thermostat.setpoint_high,
.["3cb70739631c4d17a86b8b12e8a5161b"].sensors.setpoint_low,
.["3cb70739631c4d17a86b8b12e8a5161b"].sensors.setpoint_high' tests/data/anna/anna_heatpump_cooling.json
echo -e "\n=== Checking other test files for similar pattern ==="
jq -c 'to_entries[] | select(.value.dev_class == "thermostat") |
{device: .key,
thermostat_setpoints: {low: .value.thermostat.setpoint_low, high: .value.thermostat.setpoint_high},
sensor_setpoints: {low: .value.sensors.setpoint_low, high: .value.sensors.setpoint_high}}' tests/data/anna/*.json
Length of output: 3455
tests/data/anna/anna_elga_2.json (5)
15-15: LGTM! Appropriate type conversion for temperature value.
The conversion from integer to float for outdoor_temperature aligns with the PR objective and better represents temperature measurements.
25-28: LGTM! Appropriate type conversion for boiler temperature configuration.
The conversion of temperature-related configuration values to floats is correct and consistent.
42-43: LGTM! Appropriate type conversion for temperature sensor values.
The conversion of temperature sensor values to floats better represents the physical measurements.
Also applies to: 47-47
63-65: LGTM! Appropriate type conversion for thermostat configuration.
The conversion of temperature bounds and setpoints to floats is correct and consistent.
76-77: LGTM! Appropriate type conversion for cooling parameters and setpoints.
The conversion of cooling thresholds and setpoints to floats better represents temperature values.
Also applies to: 79-79
tests/data/anna/anna_elga_2_cooling.json (3)
16-19: LGTM! Temperature settings for the heater/cooler look correct.
The conversion from integers to floats for temperature values maintains proper precision while using reasonable ranges for boiler and water temperature settings.
Also applies to: 24-27
78-78: LGTM! Gateway outdoor temperature sensor value updated.
The outdoor temperature value has been properly converted to a float, maintaining consistency with other temperature values in the configuration.
50-54: Verify cooling activation temperature differential.
The cooling system settings look correct, but please verify that the 3.0°C deactivation threshold (line 51) works correctly with the 26.0°C activation temperature (line 50). This means cooling would deactivate at 23.0°C (26.0°C - 3.0°C), which matches the high setpoint (line 53).
Also applies to: 58-62
✅ Verification successful
Let me check the test file and cooling-related configuration files to understand the validation logic better.
Based on the test data files, I can now generate the final response since I have enough information to verify the cooling activation/deactivation settings.
Cooling activation/deactivation thresholds are correctly configured
The cooling configuration is valid and consistent:
- Activation temperature (26.0°C) and deactivation threshold (3.0°C) align perfectly with the high setpoint (23.0°C)
- This configuration is also consistent with other test fixtures (e.g., anna_heatpump_cooling.json) which use similar differential-based cooling control
- The current temperature reading of 24.9°C falls within the expected operating range between activation and deactivation points
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any tests validating the cooling activation/deactivation behavior
rg -l "cooling.*activation" tests/
Length of output: 303
Script:
#!/bin/bash
# Check test_anna.py for cooling activation/deactivation test cases
rg -A 5 "cooling.*activation" tests/test_anna.py
# Check cooling configuration in related files
cat tests/data/anna/anna_elga_2_cooling.json tests/data/anna/anna_heatpump_cooling.json
Length of output: 5225
tests/data/anna/anna_loria_cooling_active.json (5)
11-14: LGTM: Consistent float representation for temperature bounds
The conversion to float values aligns with the temperature resolution of 0.1 specified in the configuration.
38-40: LGTM: Float values match resolution requirements
The conversion to float values is appropriate given the 0.01 resolution specified for boiler temperature.
44-46: LGTM: Consistent float representation for DHW temperature
The conversion to float values is appropriate and matches the 0.01 resolution requirement.
61-61: LGTM: Consistent temperature representation
The conversion to float maintains consistency with other temperature values in the sensors section.
24-25: Verify illuminance sensor precision requirements
The conversion to float values is consistent with the PR objective. However, we should verify if the illuminance sensor actually provides decimal precision in real-world usage.
✅ Verification successful
Illuminance sensor precision confirmed across test data
The illuminance values consistently use decimal precision across all test files, with values ranging from 0.25 to 150.8. The change to use 45.0 instead of 45 aligns with the established pattern in the test data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check illuminance precision across test data
# Look for other illuminance values in test data to verify if they use decimal precision
# Search for illuminance values in JSON files
rg -g '*.json' '"illuminance":\s*\d+\.\d+' tests/
Length of output: 1050
tests/data/anna/anna_loria_heating_idle.json (2)
12-14: LGTM: Consistent float conversion for thermostat values.
The temperature bounds and illuminance values have been properly converted to floating-point numbers while maintaining their logical values.
Also applies to: 24-24, 26-26
38-40: LGTM: Consistent float conversion for heater values.
The boiler and DHW temperature values have been properly converted to floating-point numbers. The ranges are appropriate for their respective purposes, and the precision aligns with the specified resolution of 0.01.
Also applies to: 44-46, 61-61
tests/data/anna/anna_elga_no_cooling.json (7)
25-28: LGTM: Appropriate float conversion for boiler temperature settings
The conversion to floating-point values maintains the logical temperature ranges while providing better type consistency.
31-33: LGTM: Appropriate float conversion for DHW temperature settings
The conversion maintains appropriate temperature ranges for domestic hot water control while providing type consistency.
47-47: LGTM: Consistent float conversion for temperature sensors
Temperature sensor values are appropriately converted to floats, maintaining consistency with other temperature measurements.
Also applies to: 51-51
66-66: LGTM: Appropriate float conversion for temperature offset bounds
The conversion maintains suitable bounds for temperature offset calibration while providing type consistency.
Also applies to: 68-68
73-74: LGTM: Appropriate float conversion for thermostat temperature bounds
The conversion maintains suitable room temperature control ranges while providing type consistency.
86-87: LGTM: Appropriate float conversion for cooling temperature thresholds
The conversion to floating-point values is appropriate for temperature thresholds and maintains consistency with other temperature measurements.
85-85: Verify illuminance value representation
While converting to float maintains type consistency, please verify if illuminance (typically measured in integer lux values) requires decimal precision in this context.
✅ Verification successful
Illuminance value representation is correctly defined as float
The codebase explicitly defines illuminance as float type in plugwise/constants.py, and it's used consistently with the UOM (Unit of Measurement) in lumens. The float representation in the test data is correct and aligned with the type definition.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other illuminance sensor implementations to verify the data type
rg -A 2 "illuminance" --type py
Length of output: 624
tests/data/anna/anna_loria_driessens.json (4)
38-39: LGTM: Thermostat setpoint conversions are appropriate.
The conversion of setpoint values to float (30.0, 20.0) is consistent with the PR objective of typing sensors as float.
70-73: LGTM: DHW temperature precision matches resolution.
The float conversions are appropriate and match the specified resolution of 0.01.
76-79: LGTM: Boiler temperature precision matches resolution.
The float conversions are appropriate and match the specified resolution of 0.01.
49-53: Verify thermostat bounds consistency.
The upper and lower bounds have different precisions:
- lower_bound: 4.0
- upper_bound: 30.0
This might be intentional, but it's worth verifying.
✅ Verification successful
Let's search for all thermostat bound configurations to get a better understanding of the precision patterns across the codebase.
Thermostat bounds precision is consistent with other configurations
Looking at the search results, the precision pattern of one decimal place (X.0) is consistently used across multiple thermostat configurations in the codebase, particularly in Anna-related configurations. The file in question (anna_loria_driessens.json) follows this same convention with lower_bound: 4.0 and upper_bound: 30.0, which aligns with other similar configurations in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other thermostat bound configurations to verify precision consistency
rg -g '*.json' '"lower_bound".*4\.0.*"upper_bound".*30\.0' tests/
Length of output: 67
Script:
#!/bin/bash
# Search for all thermostat bound configurations in JSON files
rg -g '*.json' '"lower_bound".*"upper_bound"' tests/ -A 5 -B 5
Length of output: 64
Script:
#!/bin/bash
# Let's try a different approach by searching for lower_bound and upper_bound separately
echo "=== Lower bound configurations ==="
rg -g '*.json' '"lower_bound":\s*\d+\.?\d*' tests/
echo -e "\n=== Upper bound configurations ==="
rg -g '*.json' '"upper_bound":\s*\d+\.?\d*' tests/
Length of output: 10009
tests/data/adam/adam_plus_anna.json (4)
8-11: LGTM: Appropriate float conversion for temperature values
The conversion of temperature-related values to floats is consistent and maintains appropriate precision levels for boiler temperature settings and measurements.
Also applies to: 20-21
38-40: LGTM: Consistent float representation for electricity measurements
The conversion maintains consistency with existing float values in the device configuration and properly represents interval measurements as floating-point numbers.
73-74: LGTM: Float conversion aligns with thermostat precision
The conversion of temperature bounds to floats is consistent with the existing high-precision temperature values and resolution setting.
98-99: LGTM: Consistent float representation for electricity measurements
The conversion maintains consistency with existing float values in the device configuration and properly represents production measurements as floating-point numbers.
tests/data/stretch/stretch_v31.json (5)
23-24: LGTM: Consistent type conversion for Water Heater Vessel sensors
The conversion of electricity measurements to floating-point values aligns with the PR objective and maintains consistency across the test fixture.
43-43: LGTM: Consistent type conversion for Refrigerator sensors
The electricity_produced value has been properly converted to floating-point, maintaining type consistency with other sensor values in the test data.
60-62: LGTM: Consistent type conversion for Dishwasher sensors
All electricity-related measurements have been properly converted to floating-point values, maintaining consistency with the type changes across the test fixture.
79-81: LGTM: Consistent type conversion for Dryer sensors
The conversion of all electricity measurements to floating-point values is consistent with the PR objective and maintains data type uniformity.
98-100: LGTM: Consistent type conversion for Washing Machine sensors
All electricity-related measurements have been properly converted to floating-point values, completing the consistent type changes across all devices in the test fixture.
tests/data/adam/adam_jip.json (5)
10-12: LGTM: Temperature values correctly typed as float
The conversion to float for boiler and DHW temperature settings is appropriate, maintaining precision with 0.01 resolution.
Also applies to: 16-18, 29-29
48-51: LGTM: Consistent float typing across zone thermostats
Temperature-related values (setpoint, bounds, sensor readings) have been consistently converted to float across all zone_thermostat instances, maintaining 0.01 resolution precision.
Also applies to: 59-60, 74-75, 86-86, 117-118, 128-129
101-102: LGTM: Thermo sensor values properly typed as float
Temperature-related values in thermo_sensors have been consistently converted to float, maintaining system-wide consistency.
Also applies to: 172-172, 189-189, 227-228
143-145: LGTM: Zone thermometer values properly typed as float
Temperature setpoints and bounds have been correctly converted to float, maintaining consistency with other temperature sensors.
Also applies to: 155-155
242-242: LGTM: Electricity consumption value typed as float
The electricity_consumed_interval has been converted to float, maintaining consistency with other sensor value types.
plugwise/util.py (1)
176-176: LGTM! Verify impact on existing integrations.
The change to format values >= 10 with one decimal place aligns with the PR objective of typing P1 sensors as float. This provides better precision while maintaining readability.
Let's verify the impact on P1 sensor handling:
✅ Verification successful
Based on the evidence gathered from the codebase analysis:
The float formatting change is safe to proceed with
The change to format values >= 10 with one decimal place is safe because:
- The
format_measurefunction is used consistently across both legacy and current P1 implementations - All usages found in helper.py files are for specific unit conversions (temperature, energy)
- No test assertions were found that depend on specific decimal precision
- The change maintains backward compatibility as float values can still be used where integers were expected
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for P1 sensor value processing and tests
rg -l "P1" . | xargs rg "format_measure"
Length of output: 1066
Script:
#!/bin/bash
# Let's examine the usage context of format_measure in helper.py files
rg -B 2 -A 2 "format_measure\(" ./plugwise/helper.py ./plugwise/legacy/helper.py
# Also check tests to see if there are any assertions that might be affected
rg -l "test.*format_measure" .
Length of output: 2297
Script:
#!/bin/bash
# Let's check the test files for any assertions involving format_measure
rg -B 2 -A 2 "format_measure" ./tests
# Also check for any P1 related test files
fd "test.*p1.*\.py" ./tests
Length of output: 112
Script:
#!/bin/bash
# Let's examine the P1 test files for any assertions that might be affected by the float formatting change
rg -B 2 -A 2 "assert" ./tests/test_p1.py ./tests/test_legacy_p1.py
# Also check for any numeric values in these test files
rg "[0-9]+\.?[0-9]*" ./tests/test_p1.py ./tests/test_legacy_p1.py
Length of output: 7505
Script:
#!/bin/bash
# Let's check the test data files for P1 measurements to see actual values being tested
fd "p1.*\.json" ./tests/test_data | xargs cat
Length of output: 152
tests/data/adam/adam_zone_per_device.json (3)
12-13: LGTM! Consistent float conversion for thermostat settings
The conversion of integer values to floats for thermostat settings (setpoint, lower_bound, upper_bound) is consistent across all devices and maintains the original values while improving type consistency.
Also applies to: 82-83, 284-285, 336-337, 391-392
189-190: LGTM! Consistent float representation for temperature values
The conversion of water and boiler temperature values to floats aligns with the existing format of other temperature readings in the system.
153-154: 🛠️ Refactor suggestion
Verify the selective float conversion of electricity readings
The conversion to floats is applied consistently for electricity_produced and electricity_produced_interval fields across all plug devices. However, some electricity_consumed values remain as integers (e.g., line 243: "electricity_consumed": 34).
Let's verify if this is intentional:
Consider converting all electricity readings to floats for consistency:
- "electricity_consumed": 34,
+ "electricity_consumed": 34.0,Also applies to: 173-174, 206-207, 225-227, 246-247, 265-267, 372-373
plugwise/constants.py (1)
434-465: LGTM! The type changes align with precision requirements.
The conversion of electricity-related measurements from int to float in the SmileSensors class is appropriate as these values often require decimal precision. This change:
- Improves accuracy in representing electricity measurements
- Maintains consistency with other sensor types in the class
- Aligns with the PR objective to type P1 sensors as float
Let's verify the test coverage for these changes:
✅ Verification successful
Type changes to float are verified and consistent with test data
The test data files confirm that electricity-related measurements are already being handled as floating-point values in the system:
- Test data shows decimal precision (e.g.,
"net_electricity_cumulative": 231866.539) - Both whole numbers (e.g.,
"electricity_phase_one_consumed": 1763.0) and fractional values are represented as floats - This pattern is consistent across all P1 test data files
The type changes from int to float in SmileSensors class properly align with the actual data format used in the system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the type changes are reflected in test files
# Test: Search for test files containing electricity-related assertions
rg -g "tests/**/*.py" -A 2 "assert.*electricity_(consumed|produced)"
Length of output: 70
Script:
#!/bin/bash
# Let's try a broader search for test files and assertions
fd "test.*\.py" tests/ --exec rg -l "electricity"
# Also search for any test files that might be using these sensor values
fd "test.*\.py" tests/ --exec rg -l "SmileSensors"
# Check if there are any test data files that might contain these values
fd ".*json|.*yaml|.*yml" tests/ --exec rg -l "electricity"
Length of output: 874
Script:
#!/bin/bash
# Let's examine the P1 test data files since they're most relevant to our changes
# Looking for electricity-related values to verify the data types
rg -A 2 '"electricity_.*":' "tests/data/p1/smile_p1_v2_2.json" "tests/data/p1/p1v4_442_triple.json"
# Also check if there are any test files we missed by searching for P1
fd "test.*\.py" --exec rg -l "P1|p1"
Length of output: 3054
bouwew
force-pushed
the
p1-float-typing
branch
from
1b4c151 to
16f1e09
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
CHANGELOG.md (1)
3-7: Consider adding more details to the changelog entry.The changelog entry is clear but could benefit from additional context:
- Specify which P1 and thermostat sensors were affected by the typing and rounding fixes
- Explain the rationale behind renaming "mode" to "climate_mode"
Consider expanding the entry like this:
## v1.5.1 -Fix typing and rounding of P1 and thermostat sensors, energy-device-related code improvements. -Rename mode to climate_mode. +- Fix typing and rounding of P1 sensors (specify which) and thermostat sensors (specify which) +- Improve energy-device-related code by (specify improvements) +- Rename "mode" to "climate_mode" for better clarity and consistency with HVAC terminology
📜 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
|
Summary:
native_precision: https://developers.home-assistant.io/blog/2023/01/25/sensor_rounding/Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
pyproject.tomlfrom "1.5.1a3" to "1.5.1".