goecharger / go-eCharger-API-v1

API specification for V2 go-eCharger (compatible with V3 too)
100 stars 26 forks source link

Why are numbers represented as JSON string values ? #4

Open rhuss opened 4 years ago

rhuss commented 4 years ago

A lot of numeric values are presented as JSON string values like in

{
  "version": "B",
  "tme": "2808201703",
  "rbc": "18",
  "rbt": "607477507",
  "car": "1",
  "amp": "16",
  "err": "0",
  "ast": "0",
  "alw": "1",
  "stp": "0",
  "cbl": "20",
  "pha": "56",
  "tmp": "16",
  "tma": [
    24.12,
    23.5,
    24,
    23.62
  ],
  "amt": "32",
  "dws": "0",
  "dwo": "0",
  "adi": "1",
  "uby": "0",
  "eto": "780",
  "wst": "3",
  "txi": "2",
  "nrg": [
    216,
    218,
    216,
    1,
    0,
    0,
    0,
    0,
    0,
    0,
    0,
    0,
    0,
    0,
    0,
    0
  ],
  "fwv": "033",
  "sse": "014371",
...
  "loe": 0,
  "lot": 0,
  "lom": 0,
  "lop": 0,
  "log": "",
  "lon": 0,
  "lof": 0,
  "loa": 0,
  "lch": 10496
}

Why is this the case and could this be changed ? This "convention" makes it hard to import the data e.g. in InfluxDB with a Telegraf importer.

Please note that other values are presented as numbers like the nrg array.

Interestingly e.g. both loa and amp are described as of type uint8_t but one is represented as a JSON number the other as a JSON string. It should be at least consistent, preferably all numbers should be also JSON numbers.

IPSCoyote commented 3 years ago

Please keep in mind: A change can corrupt existing implementations and would be potentially incompatible

rhuss commented 3 years ago

Sure. API evolution is a common requirement, though and there are many solutions to implement those (e.g. encoding the API version in the endpoint path itself e.g. a /v2/... so that a client can select its endpoint, older clients can still access the old values.

The purpose of this issue to point out that inconsistency and unexpected usage of JSON, so might be considered to be fixed in a future version of the API (and not to change it unconditionally right now).

KaiDK commented 3 years ago

This "convention" makes it hard to import the data e.g. in InfluxDB with a Telegraf importer.

Full ack. Had the same problem importing into Telegraf!

It should be at least consistent, preferably all numbers should be also JSON numbers.

I also would highly appreciate this!

IPSCoyote commented 3 years ago

It's a released API. I highly recommend NOT to change it in an incompatible way as this would or might break existing integrations.

peterpoetzi commented 3 years ago

I am sorry that the design choice for the String encapsulated integer is not convenient to use.

However, we will keep it as it is, but if we release a v2 version of the API, there will be no String encapsulated Integers anymore.

geowiwi commented 3 years ago

+1 for releasing a v2 API without encapsulated integers.

Currently, important parameters like dws and eto are also not transmitted via MQTT, while unencapsulated integers are transmitted. The workaround requires http overhead and, as mentioned before, makes processing in telegraf unnecessary difficult.

[[inputs.http]]
urls = [
    "http://goe"
  ]
  json_query = ["dws","eto"]
  tagexclude = ["url", "host"]
  name_override = "goe"
  headers = {"cache-control" =  "no-cache","content-type" =  "application/json"}
  data_format = "json"
  tag_keys = ["dws","eto"]

[snip]

[processors]
  [[processors.converter]]
    [processors.converter.tags]
      integer = ["dws","eto"]
rhuss commented 3 years ago

fyi @geowiwi I still was able to import the data with telegraf and the mqtt transport via that configuration:

[[inputs.mqtt_consumer]]
  name_override = "go-echarger"
  servers = ["tcp://broker.mqtt:1883"]
  topics = [
    "go-eCharger/+/status",
  ]
  topic_tag = "topic"
  qos = 1
  username = "...."
  password = "...."

  data_format = "json"
  json_query = ""

  json_time_format = "unix"
  json_timezone = "Europe/Berlin"

  tag_keys = [
    "sse"
  ]

  json_string_fields = ["dws", "amp", "eto", "tmp" ]

[[processors.converter]]
  namepass = "go-echarger"
  [processors.converter.fields]
    float =  ["dws", "amp", "eto", "tmp" ]