dbus-mqtt: send null instead of empty array for dbus-invalid

[demaniak]

I updated to version 2.85 today.
All went well, and my MQTT integration seemed happy.

Then tonight we got hit with mains outage again (or as it is colloquially know, "load shedding").
And things went a bit weird.
For topic N/xxxxxxxxxxxx/system/0/Ac/ConsumptionOnInput/L1/Power, a payload like {"value": 555} is typically expected.

However, what was actually received, was {"value": []}.

So fair enough, mains are dead, so no reading - but switching from a numeric type to a list type is a bit drastic, don't you agree?

I believe in earlier versions I would get a good solid 0 (zero) there instead, which is perfectly acceptable.

Once mains power was restored, I received the expected payload again.

[demaniak]

Just to add and demonstrate - VRM is also not liking non-essential loads values during mains-outage:

Note the NaN value there - surely because [] is not a number....

Side note: dark mode rocks, thanks to everybody that worked/is-working on that :)

[jhofstee]

yes, values do become undefined / null whatever when not known. On the dbus we do use a empty array for that. If there are better implementations for json please let us know.

[demaniak]

Hi @jhofstee , thanks or the reply :)

Completely understand the problem - you don't HAVE a value, so how do you communicate that to the consumer?

First, just to reiterate the problem I hit.

In a typed language binding situation, I'm expecting a numeric value. Now, under this set of conditions the type of the value suddenly changes from numeric to a list type (or array, or collection, whatever you want to call it).
This results in some disruptions when automatic de-serialization occurs. It can all be handled (kinda clumsily) if you know about it.

So the question then for me is: what is the "right" thing to do?
Personally, I would lean towards the principle of "least surprise" where a change in value type is probably almost the most surprising thing that could happen.

The options I can think of, in order or least to most surprising is:

1. a value of 0 (zero) (I THINK this is what previous versions of VenusOS did?)
2. a value of null
3. an empty object as message (i.e. value key is completely removed)

The above options should not break de-serialization, but would still express the current situation.

0 (zero) MIGHT be misleading if a consumer is not looking at more topics.
The original json spec allows for null as a value.

There is a nice StackOverflow post here on the topic.

It would be great if any other MQTT consumers / integrators could also throw in some opinions :)

[thepaulcooper]

If there is no value then my vote would be for it to be null.

[demaniak]

I would be happy with null as well.

The good:

1. makes for clear distinction between 0(zero) value,and "no value available'
2. should not break due to type changes when deserializing

The bad:

1. nulls have a way of causing null pointer exceptions if you are not careful - but you should be treating incoming data with extreme caution anyway, so not a major concern for the cautions integrator.

[mpvader]

Hi all,

rather old open issue; sorry for that.

I checked with several developers (VRM, VictronConnect) and they see no immediate issue with changing this. And agree that its technically better.

We’ll still take this slowly, since changing this needs testing on various apps where we use this data; I know at least three:

• html5 app
• VictronConnect
• VRM Portal

and then there is

• node-red
• Signalk venus plugin: no issue for that, since it converts empty array to null already
• Grafana docker venus

And then there are customer implementations to consider.

lots of work and care for this seemingly small change.

so; lets see! At least you know where aware of this.

[demaniak]

Thank you @mpvader - quality takes time :)

[wiebeytec]

@demaniak do you get { "value": [] } on other topics as well? An empty dbus array is indeed used for invalid values, but this line of code should always make that null on mqtt. It seems this line and maybe the default exit are the only places that could turn that into an actual array/list, and those are not supposed to be called, as far as I can tell.

Also interesting, are there topics that already do say {"value":null}?

I ask, because I can't reproduce it (meaning I get null on invalid values). This means you may have found another bug.

[demaniak]

@wiebeytec , let me try and answer :)

do you get { "value": [] } on other topics as well?

Off the cuff, I can not say. My integration does not subscribe to ALL available topics - I have only noticed this particular situation for the N/xxxxxxxxxxxx/system/0/Ac/ConsumptionOnInput/L1/Power topic.

Also interesting, are there topics that already do say {"value":null}?

Again, I'm unfortunately not sure - I have not looked closely at this code for a while - it is working and doing what I want, so further changes have been slow :)

I'm pretty sure I see some empty string values (i..e "", not null) for at least two two other topics - this is typically shortly after a MQTT connection restart (i.e. restart of the integration app).

The code has always been pretty defensive regarding null, so it could be that I just ignore those.

If you think it can help, I can maybe add some checking and logging for null values, and make sure what I'm seeing.

BUT, I will also just mention that I can't really logically see that null would make sense for the other topics I'm currently subscribing to.
For example:

• solarcharger/258/Yield/Power
• system/0/Dc/Battery/Power
• system/0/Ac/ConsumptionOnInput/L1/Power

The only why I could understand a null from those topics would be in the case of catastrophic failure, or removal of PV Panels/Battery .

Maybe system/0/Ac/ConsumptionOnInput/L1/Power could do that if the earth leakage trips, but in that case everything goes down on the integration side, so .. testing that is a bit tricky.

I ask, because I can't reproduce it (meaning I get null on invalid values). This means you may have found another bug.

Well.. maybe let me try and be more specific about the state I observe this in.

So when the system/0/Ac/ActiveIn/Source topic value goes to anything other than 1, in other words there is a mains (grid) outage detected, this is when I see this.

On the VRM this can be seen as the Ac Loads value being displayed as NaN, which is correct, a [] is most certainly not a number.

Maybe also just worth checking current Venus versions we are talking about?
I'm currently on v2.94

[mpvader]

Hey @demaniak thank you for the details. Wiebe and myself discussed this just now, here is the result:

• We're in the middle to change our mqtt implementation in Venus OS. Details of that are here: Replace Mosquitto with FlashMQ #1098.
• During that, we'll make sure that proper null values are used; also in the topics you gave as examples.

I'll close this issue; to keep it central. Thanks!