search

screekworkshop / screek-human-sensor

137 stars 29 forks source link

2A Zone configuration warning/error #8

Closed Cossid closed 1 year ago

Cossid commented 1 year ago

https://github.com/screekworkshop/screek-human-sensor/blob/9a237b7355bdedb31a9a43a1c4b1692838a3e595/2a/yaml/human-sensor-2a-stable-github.yaml#L532

Is throwing a warning

/2a/yaml/human-sensor-2a-stable/github.yaml:532:32: warning: left operand of comma operator has no effect [-Wunused-value] 532 | if (id(zone1_x_begin).state == 0, id(zone1_x_end).state == 0, id(zone1_y_begin).state == 0, id(zone1_y_end).state == 0){ | ~~~~~^~~~ /2a/yaml/human-sensor-2a-stable/github.yaml:532:57: warning: right operand of comma operator has no effect [-Wunused-value]

which means it is being processed with a warning as if (id(zone1_x_begin.state == 0)) { and not working as designed

I'm guessing this is supposed to be && instead of commas.

The same is happening for zone2 on line 554, and zone3 on line 576

As a result, only having X Begin unset causes it to be listed as zone unconfigured.

screekworkshop commented 1 year ago

It looks like there was a bug caused by a misspelling, if '(id(zone1_x_begin).state == 0)' This would fit the original logic. Thanks for the feedback.

Cossid commented 1 year ago

I believe the intended logic is supposed to be if (id(zone1_x_begin).state == 0 && id(zone1_x_end).state == 0 && id(zone1_y_begin).state == 0 && id(zone1_y_end).state == 0){, but I didn't want to assume. I'd guess if all 4 values are unset, it would be considered unconfigured, but if any one is set, it would be configured. As written with commas, the comma is an unexpected syntax, and c++ is trying to correct it by ignoring anything to the right of it, so it is only checking the first value, so a setting of x_begin 0 and x_end of 10 would be considered unconfigured, for example.

Note, I haven't used this, hence I don't want to make assumptions, just helping someone else trying to use it.

screekworkshop commented 1 year ago

I see what you're saying, it leads to an incorrect subcode calculation logic.
Since I use apple's swift language a lot, commas tend to be used for some weird splicing in there.
We will be tweaking this syntax.

screekworkshop commented 1 year ago

it's has fix with new code https://github.com/screekworkshop/screek-human-sensor/commit/d9ab720b14f321a058c11b769bd332f321a81711

thx:)