showwin / speedtest-go

CLI and Go API to Test Internet Speed using speedtest.net
MIT License
565 stars 115 forks source link

json output never uses Mbps format for speeds #219

Closed luevano closed 5 months ago

luevano commented 5 months ago

Suddenly after some update I stopped getting speed outputs in Mbps and instead I'm receiving something else (not sure which of the 4 it is).

Outputs before: DL: 304.5242 UL: 310.7918

Outputs currently: DL: 37333361.36173324 UL: 37774782.821372464

I tried using all 4 different --unit options and also just doing "Mbps", because according to this it should use the default Mbps format:

https://github.com/showwin/speedtest-go/blob/8e021445a08fc4a1fcc9f454711ca1d233cad745/speedtest.go#L54

https://github.com/showwin/speedtest-go/blob/8e021445a08fc4a1fcc9f454711ca1d233cad745/speedtest.go#L292-L305

https://github.com/showwin/speedtest-go/blob/8e021445a08fc4a1fcc9f454711ca1d233cad745/speedtest/unit.go#L58

Speedtest version (from AUR): speedtest-go v1.7.7 git-dev built at unknown

luevano commented 5 months ago

Looks like ByteRate.String() is never called.

r3inbowari commented 5 months ago

Hi, @luevano, I think we should keep the status quo, as the purpose of the current changes is to be consistent with Ookla.

Looks like ByteRate.String() is never called.

I want to explain why it is never called on json.

// Cited from Ookla speedtest Machine readable formats (csv, tsv, json, jsonl, json-pretty) use bytes as the unit of measure with max precision

It has also been explained here: https://github.com/showwin/speedtest-go/pull/195

So, this is what we need to clarify:

  1. --unit is designed to be human-readable only.
  2. JSON as a machine-readable language, should not be mixed with human-readable units, which is unnecessary. Because it makes the reprocessing of JSON complicated.

Additional content reference(private content has been removed):

  1. speedtest-go json format:
{
  "servers": [
    {
      "dl_speed": 15220296.163629018,
      "ul_speed": 2605123.5026477408,
      "test_duration": {
        "ping": 2504038600,
        "download": 10050985300,
        "upload": 13714513500,
        "total": 26269537400
      },
      "packet_loss": { "sent": 317, "dup": 0, "max": 332 }
    }
  ]
}
  1. Ookla speedtest json format:
{
  "type": "result",
  "ping": { "jitter": 2.895, "latency": 28.122, "low": 27.453, "high": 37.423 },
  "download": {
    "bandwidth": 12968587,
    "bytes": 187972978,
    "elapsed": 15012,
    "latency": { "iqm": 32.207, "low": 20.373, "high": 278.771, "jitter": 17.591 }
  },
  "upload": {
    "bandwidth": 10971637,
    "bytes": 43506137,
    "elapsed": 4025,
    "latency": { "iqm": 41.782, "low": 21.085, "high": 159.054, "jitter": 19.7 }
  }
}

Currently, the bandwidth unit of both are consistent(bytes per sec) in json output, except that speedtest-go uses float type.

luevano commented 5 months ago

// Cited from Ookla speedtest Machine readable formats (csv, tsv, json, jsonl, json-pretty) use bytes as the unit of measure with max precision

It has also been explained here: #195

So, this is what we need to clarify:

1. `--unit` is designed to be human-readable only.

2. JSON as a machine-readable language, should not be mixed with human-readable units, which is unnecessary. Because it makes the reprocessing of JSON complicated.

Got it, wasn't aware of the "requirements". This and the PR can be closed then.

r3inbowari commented 5 months ago

Anyway, thank you for your PR. Hope next!