fix: Use correct SI units for speed test conversion - #629
Merged
Conversation
Review Summary by QodoFix network speed calculations to use correct SI units
WalkthroughsDescription• Replace binary conversion (×1024) with SI conversion (×1000) for network speed calculations • Change kilobit unit from "Kb" to "kb" per SI standard lowercase prefix • Update test expectations to match corrected SI unit formatting Diagramflowchart LR
A["Binary conversion ×1024"] -->|"Replace with"| B["SI conversion ×1000"]
C["Unit: Kb"] -->|"Change to"| D["Unit: kb"]
B --> E["Accurate speed display"]
D --> E
File Changes1. lib/utils.dart
|
Code Review by Qodo
1. Chart uses 1024 base
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
188
to
195
| final download = NetworkUtils.formatBitsWithUnit( | ||
| (speedTestResult.downloadBandwidth ?? 0) * 1024, | ||
| (speedTestResult.downloadBandwidth ?? 0) * 1000, | ||
| decimals: 1, | ||
| ); | ||
| final upload = NetworkUtils.formatBitsWithUnit( | ||
| (speedTestResult.uploadBandwidth ?? 0) * 1024, | ||
| (speedTestResult.uploadBandwidth ?? 0) * 1000, | ||
| decimals: 1, | ||
| ); |
There was a problem hiding this comment.
1. Chart uses 1024 base 🐞 Bug ✓ Correctness
After this PR’s switch to base-1000 conversions for speed-test bandwidth, the dashboard history chart still divides Kbps by 1024 to get Mbps and uses a brittle unit-string fallback. This will make historical chart values inconsistent with the live speed-test UI and can be plainly wrong when the raw Kbps fields are absent/zero.
Agent Prompt
### Issue description
The speed test UI now treats bandwidth as SI (base-1000) (e.g., Kbps→bps uses `* 1000`, and `NetworkUtils.formatBitsWithUnit` uses base-1000 and returns `unit` like `kb`). The dashboard history chart still converts Kbps→Mbps using `/ 1024.0` and only converts parsed values when `unit.toUpperCase() == 'KBPS'`, which does not match the unit strings actually stored/produced.
### Issue Context
This causes historical chart values to diverge from the live speed-test UI. It can also produce plainly wrong values in the fallback branch when `downloadBandwidthKbps/uploadBandwidthKbps` are null/0, because the chart may skip conversion and “assume Mbps”.
### Fix Focus Areas
- lib/page/dashboard/views/components/widgets/atomic/speed_test.dart[438-467]
- lib/utils.dart[330-352]
- lib/page/health_check/services/health_check_service.dart[176-209]
### Suggested fix
1. In the chart, change Kbps→Mbps conversion to divide by `1000.0` (not `1024.0`) for both download and upload.
2. Replace the `unit == 'KBPS'` fallback with logic that recognizes the actual units used by the app (case-insensitive):
- `kb` / `kbps` => divide by 1000
- `mb` / `mbps` => no change
- `gb` / `gbps` => multiply by 1000
- (optionally extend for Tb/Pb)
3. Consider centralizing this conversion into a small helper (e.g., `double toMbps(double value, String unit)`), and add a unit test for it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
AustinChangLinksys
force-pushed
the
fix/speed-test-si-units
branch
from
February 11, 2026 13:05
e4dca70 to
9c88dfe
Compare
- Change "Kb" to "kb" for kilo prefix (SI standard uses lowercase k) - Replace binary conversion (* 1024) with SI conversion (* 1000) for Kbps to bps - Fix dashboard history chart to use SI conversion (/ 1000.0) and handle unit variations - Update related test expectations Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AustinChangLinksys
force-pushed
the
fix/speed-test-si-units
branch
from
February 11, 2026 13:12
9c88dfe to
5584eb9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
* 1024) with SI conversion (* 1000) for Kbps to bpsDetails
Network speeds should use SI units (base 1000), not binary units (base 1024):
The previous implementation caused displayed values to be ~2.4% higher than actual.
Test plan
flutter test test/utils_test.dart --name "formatBits"- all 17 tests pass🤖 Generated with Claude Code