Pseudo nesting clarification

[DigiH]

Hi @h2zero, having gone through the pseudo-nesting implementation together for the Xiaomi scales a while back you have since also implemented proper nesting for model conditions, but in the docs added a note

Nesting is discouraged …

with the example

"condition": ["servicedata", "index", 30, "abcd", "|", "servicedata", "index", 0, "1234", "&" "servicedata", "index", 5, "5678"]

stating that this works the same as a properly nested

"condition": [["servicedata", "index", 0, "1234", "&" "servicedata", "index", 5, "5678"], "|", "servicedata", "index", 30, "abcd"]

Having just confirmed a similar case with my open b-parasite PR and even linking to an old Xiaomi scale implementation I am a bit confused now ;) as

"condition": ["servicedata", "index", 30, "abcd", "|", "servicedata", "index", 0, "1234", "&" "servicedata", "index", 5, "5678"]

should work the same as

"condition": [["servicedata", "index", 30, "abcd", "|", "servicedata", "index", 0, "1234"], "&" "servicedata", "index", 5, "5678"]

but not as the above stated nested example in the docs.

Just to be sure I have made a few quick tests with how the pseudo-nesting still works.

Can you confirm my findings and that the docs should be amended?

Isn't it the case that the properly nested example cannot actually be accomplished without nesting, hence you implemented the proper nesting, but my above example allows for pseudo-nesting in such cases, where proper nesting can be avoided.

Thanks

Suggested changes:

https://github.com/DigiH/decoder/commits/docs-nesting-changes

[h2zero]

Hi @DigiH,yes those should work the same. The OR cases would be checked first and if any matched the code will loop through the conditions to check for an AND case, then check that if found. At least that's the way it's intended to work.

The key is that the AND case should be the last case to avoid the nesting requirement.

[DigiH]

Hi @h2zero, thanks for the reply, but getting even more confused now ;)

So you say that as it currently says that

"condition": [["servicedata", "index", 0, "1234", "&" "servicedata", "index", 5, "5678"], "|", "servicedata", "index", 30, "abcd"]

can also be written as

"condition": ["servicedata", "index", 30, "abcd", "|", "servicedata", "index", 0, "1234", "&" "servicedata", "index", 5, "5678"]

is correct?

Where I think, and as I think you confirmed in your last post

The OR cases would be checked first and if any matched the code will loop through the conditions to check for an AND case

it won't match the same, as the latter, with the AND correctly at the end, does not allow for the single
"servicedata", "index", 30, "abcd"

condition match of the first properly nested example. Therefor my suggested changes.

[h2zero]

Yes you're correct, they are not equal, but it depends on context. At the time the nesting was added the documentation was correct in the context we were working with. So it really depends on what you're trying to do 😄

it won't match the same, as the latter, with the AND correctly at the end, does not allow for the single
"servicedata", "index", 30, "abcd"

True, it should be the first condition checked then OR [nested condition] for efficiency.

If you need if((x || y) && z) then the nesting can be omitted.
If you need if((x && y) || z) then nesting would be required.

It's confusing for sure and the docs should have more detail about the subject.

[DigiH]

If you need if((x || y) && z) then the nesting can be omitted.

That's what I've got in my changes in the NOTE section now with the comment that it only applies when ALL conditions in a bracket are chained with OR, and leaving the above if((x && y) || z) example with proper nesting.

Unless you think that needs further explanaintion as well I will submit my changes.

It's confusing for sure and the docs should have more detail about the subject.

That's the premise I've been working with all along, so was pretty clear to em, just got confused when I saw the contradicting docs when I revisited them recently ;)

[h2zero]

Any details you can think of to avoid the confusion would be great to add to the docs.

Maybe some examples of when and how to use nesting would help as well.

[DigiH]

I'm a bit embarrassed to say, especially since it was my requests who got you started on implementing the nesting, with or without bracketing, that currently, with all other new possibilities we only need the unbracketed nesting and don't actually have a single case which requires proper bracketed nesting. But great to have the possibility if needed for future decoders.

I'll think of a further explanation to add the the bracketed example by tomorrow and then submit a PR for you to review ;)

[h2zero]

That's also why the documentation is lacking. I didn't see any use for it yet so I skimped on the doc when it was added 😀.

One day it may be useful though, never a bad thing to have more tools.

[DigiH]

Just created a PR, see if you think it's clear enought or should still be extended :)