Bug 433

Summary:	DataRate: kBps and kB/s have wrong multipliers.
Product:	ns-3	Reporter:	TimoB <timo.bingmann>
Component:	core	Assignee:	Tom Henderson <tomh>
Status:	RESOLVED FIXED
Severity:	major	CC:	craigdo, gjcarneiro, mathieu.lacage, raj.b, riley, tomh
Priority:	P1
Version:	ns-3-dev
Hardware:	All
OS:	All
Attachments:	data-rate.cc and data-rate.h patch revised, cleaned-up patch for units tom's patch revised again

Description TimoB 2008-12-05 08:16:28 UTC

Obviously wrong multipliers in DataRate::DoParse().

Patch and some doxygen comments:
https://idlebox.net/2008/ns-3-wifi/code/ns-3-wifiex/rev/5dfb8b3b6210

By the way: I somehow expected 1 kBps to be 1024 Bps and not 1000 Bps. Is there are reason for the 1000 multiplier?

Comment 1 Gustavo J. A. M. Carneiro 2008-12-05 10:58:00 UTC

-1

I know this is confusing, but IMHO 1 kbit/s == 1000 bit/s, and 1 kB == 1024 byte.
At least this is how traditionally network engineers deal with the unit prefixes; when dealing with bitrates, k means 1000, when dealing with information (bytes) k means 1024.

Comment 2 TimoB 2008-12-05 11:10:27 UTC

It's ok, just needs a big highlighted doxygen note.

Maybe even add the (strange) KiB/s MiB/s GiB/s variants with 2^x multipliers.

I hope it is clear that the two multipliers for kBps and kB/s are still very wrong.

Comment 3 Tom Henderson 2008-12-05 11:56:00 UTC

Created attachment 327 [details]
data-rate.cc and data-rate.h patch

The attached patch tries to align it with:
http://en.wikipedia.org/wiki/Mbit/s#.27k.27_vs_.27Ki.27

(I only revised the k/K/Ki units in this patch, but similar could be done for the other prefixes)

Comments?

Comment 4 TimoB 2008-12-05 12:17:31 UTC

Good. Don't forget G and Gi in the doxygen.

Comment 5 Tom Henderson 2008-12-10 00:42:09 UTC

Created attachment 329 [details]
revised, cleaned-up patch for units

this finishes off alignment with:
http://en.wikipedia.org/wiki/Mbit/s#.27k.27_vs_.27Ki.27

Basically, there shouldn't be much change to users except a few more strings are supported, and the two errors that Timo pointed out are fixed, and doxygen is added.

Comment 6 Mathieu Lacage 2008-12-10 05:36:32 UTC

(In reply to comment #5)
> Created an attachment (id=329) [details]
> revised, cleaned-up patch for units
> 
> this finishes off alignment with:
> http://en.wikipedia.org/wiki/Mbit/s#.27k.27_vs_.27Ki.27
> 
> Basically, there shouldn't be much change to users except a few more strings
> are supported, and the two errors that Timo pointed out are fixed, and doxygen
> is added.
> 

patch looks great modulo a set of spurious \n which are present in the API doc.

The API doc looks great !

Comment 7 TimoB 2008-12-10 07:30:50 UTC

Created attachment 331 [details]
tom's patch revised again

Changes: MBps was duplicate. KiB/s, Kib/s etc. are the unit suffixes not "Kibyte/s".

Comment 8 Tom Henderson 2008-12-10 10:44:16 UTC

(In reply to comment #7)
> Created an attachment (id=331) [details]
> tom's patch revised again
> 
> Changes: MBps was duplicate. 

agreed, thanks.

> KiB/s, Kib/s etc. are the unit suffixes not
> "Kibyte/s".
> 

I do not care strongly.  Both are discussed in the wikipedia page I referenced and I just picked one.  These units may never be used, so I think it is OK to just support one of the string formats.  I'm fine with your revised patch, thanks.