Skip to content
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

Results interface may incorrectly reports throughput units #6

Open
nkinkade opened this issue Jan 23, 2019 · 3 comments
Open

Results interface may incorrectly reports throughput units #6

nkinkade opened this issue Jan 23, 2019 · 3 comments

Comments

@nkinkade
Copy link
Contributor

A user reported that their internet connection is supposed to be 12Mbps down and 1Mbps up, but, when testing, the interface reports the upload speed as "0.92 kb/s". They flagged this as a probable bug in our code, and that it likely should be reading "0.92 Mb/s".

Looking at the code that renders this display value, it does appear that there is an issue. When determining the unit for the speed result the code assumes that the value it is working with is Kb/s. However, the value that is getting passed into that function, has already been converted (roughly) from Kb/s to Mb/s.

The easy fix for this, I believe, is to simply stop using the getSpeedUnit() function and to fix the unit Mb/s. Speeds are advertised, usually, as Mbps, and so, even if this worked, reported Kbps or Gbps (Tbps?) may not be useful to a user.

@pboothe
Copy link

pboothe commented Jan 30, 2019

The conversion you see from Kb to Mb ( https://github.com/m-lab/mlab-speedtest/blob/master/app/assets/js/ndt-wrapper.js#L181 ) is, I thought, actually a conversion from b to Kb?

@nkinkade
Copy link
Contributor Author

@pboothe
Copy link

pboothe commented Jan 31, 2019

You win! I had forgotten about that /1000 in there.

I think your solution is correct, but it did not remove enough code - we also need to delete the use of the "justified" function. I'll comment further there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants