-
-
Notifications
You must be signed in to change notification settings - Fork 369
fix #4150: display 0°C instead of -0°C for near-zero negative temperatures
#4186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix #4150: display 0°C instead of -0°C for near-zero negative temperatures
#4186
Conversation
Fixes the negative zero display bug where temperatures between -0.4°C and -0.1°C would incorrectly show as \"-0°C\". The fix uses absolute value rounding with separate sign handling to ensure values that round to zero always display without a negative sign. Also adds comprehensive unit tests for temperature formatting including: - Data-driven test cases for various temperature values - Boundary tests around -0.5°C rounding threshold - Fahrenheit conversion tests - Dew point calculation tests Fixes meshtastic#4150
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #4150 where temperatures between -0.4°C and -0.1°C would incorrectly display as -0°C instead of 0°C. The fix implements a more careful rounding approach by handling the absolute value and sign separately.
Changes:
- Updated
toTempString()function to round absolute temperature values and handle the sign separately to avoid negative zero display - Added comprehensive unit tests covering the bug scenario, boundary cases, and Fahrenheit conversions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
core/model/src/main/kotlin/org/meshtastic/core/model/util/UnitConversions.kt |
Refactored toTempString() to use absolute value rounding with separate sign determination to prevent -0°C display |
core/model/src/test/kotlin/org/meshtastic/core/model/util/UnitConversionsTest.kt |
Added comprehensive unit tests for temperature formatting including edge cases, boundary conditions, and Fahrenheit conversions |
| val roundedAbsoluteTempt: Int = absoluteTemp.roundToInt() | ||
|
|
||
| val isZero = roundedAbsoluteTempt == 0 | ||
| val isPositive = kotlin.math.sign(temp) > 0 | ||
| val sign: String = if (isPositive || isZero) "" else "-" | ||
|
|
||
| return "$sign$roundedAbsoluteTempt°$unit" |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same typo appears here: roundedAbsoluteTempt should be roundedAbsoluteTemp to match the intended variable name and maintain consistency.
| val roundedAbsoluteTempt: Int = absoluteTemp.roundToInt() | |
| val isZero = roundedAbsoluteTempt == 0 | |
| val isPositive = kotlin.math.sign(temp) > 0 | |
| val sign: String = if (isPositive || isZero) "" else "-" | |
| return "$sign$roundedAbsoluteTempt°$unit" | |
| val roundedAbsoluteTemp: Int = absoluteTemp.roundToInt() | |
| val isZero = roundedAbsoluteTemp == 0 | |
| val isPositive = kotlin.math.sign(temp) > 0 | |
| val sign: String = if (isPositive || isZero) "" else "-" | |
| return "$sign$roundedAbsoluteTemp°$unit" |
| val unit = if (isFahrenheit) "F" else "C" | ||
|
|
||
| // Convoluted calculation due to edge case: rounding negative values. | ||
| // Rounding `-0.5` gives: `0`, since we're rounding `up`. We want `-0.5` to be rounded to `-1`. |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment mentions "Rounding -0.5 gives: 0, since we're rounding up", but this is inaccurate. Kotlin's roundToInt() uses "round half to even" (banker's rounding) by default, and for -0.5, the absolute value is 0.5, which would round to 0. However, the statement "since we're rounding up" is misleading because the rounding behavior being described is for the absolute value, not the original negative number. Consider clarifying that you're working with the absolute value to avoid confusion about whether -0.5 rounds to 0 or -1.
| // Rounding `-0.5` gives: `0`, since we're rounding `up`. We want `-0.5` to be rounded to `-1`. | |
| // We round the absolute value using roundToInt() (banker's rounding), then reapply the sign so values |
| val roundedAbsoluteTempt: Int = absoluteTemp.roundToInt() | ||
|
|
||
| val isZero = roundedAbsoluteTempt == 0 | ||
| val isPositive = kotlin.math.sign(temp) > 0 | ||
| val sign: String = if (isPositive || isZero) "" else "-" | ||
|
|
||
| return "$sign$roundedAbsoluteTempt°$unit" |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the variable name: roundedAbsoluteTempt should be roundedAbsoluteTemp (missing the final 'p'). This typo makes the variable name inconsistent and potentially confusing.
| val roundedAbsoluteTempt: Int = absoluteTemp.roundToInt() | |
| val isZero = roundedAbsoluteTempt == 0 | |
| val isPositive = kotlin.math.sign(temp) > 0 | |
| val sign: String = if (isPositive || isZero) "" else "-" | |
| return "$sign$roundedAbsoluteTempt°$unit" | |
| val roundedAbsoluteTemp: Int = absoluteTemp.roundToInt() | |
| val isZero = roundedAbsoluteTemp == 0 | |
| val isPositive = kotlin.math.sign(temp) > 0 | |
| val sign: String = if (isPositive || isZero) "" else "-" | |
| return "$sign$roundedAbsoluteTemp°$unit" |
|
Bot's comments look reasonable, please make the changes. And I definitely appreciate the added tests ! |
Summary
Fixes the negative zero temperature display bug, where values between
-0.4°Cand-0.1°Cwould incorrectly show as-0°C.Fixes #4150
Changes
Modified files:
core/model/src/main/kotlin/org/meshtastic/core/model/util/UnitConversions.kt- UpdatedtoTempString()to use absolute value rounding with separate sign handlingcore/model/src/test/kotlin/org/meshtastic/core/model/util/UnitConversionsTest.kt- Added unit tests for temperature conversionImplementation details:
The
toTempString()function now:This approach handles the edge case where
%.0fformatting produces-0for values between -0.4 and -0.1.Testing
Added unit tests including:
Run tests:
./gradlew :core:model:testGoogleDebugUnitTest --tests "org.meshtastic.core.model.util.UnitConversionsTest"