The "allow" meme in scene scripts must die!

There has long been a code fragment floating around these forums, and it’s broken in a somewhat unobvious (to many) way. While I’ve noticed this in the past when the code fragment has been reposted, I’ve never really chimed in to point out the error (that little fellow on my shoulder whispering “stay out of it, rigpapa”), but in light of other recent circumstances, I now realize that that choice was an error.

So, here is my treatise on what I will call “the allow meme” that has become ubiquitous in people’s scene Lua code here in the community.

Background… we’re talking about variants of this particular code fragment, intended to be used in scene Lua as a way to unconditionally turn off the scene actions under certain conditions. That is, the intent is that setting (explicitly or by formula) allow = false will have the scene actions not execute regardless of the result of an accompanying test condition. Here’s typical example code using temperature, although time of day is also commonly seen:

-- Run scene actions only when temp between 68 and 72 and virtual switch is ON (never runs when switch OFF).
local tCurrent = luup.variable_get( "urn:upnp-org:serviceId:TemperatureSensor1", "CurrentTemperature", someTempSensor )
local tLow = 68
local tHigh = 72
-- allow: scene runs when true, false blocks it [yeah, no, it doesn't. Keep reading.]
local allow = luup.variable_get( "urn:upnp-org:serviceId:SwitchPower1", "Status", someVirtualSwitch ) == "1"
return (((tCurrent >= tLow) and (tCurrent <= tHigh)) == allow)

Seems legit. When allow is true, in this case when someVirtualSwitch is on, then this Lua fragment will return true to the scene when the current temperature reading is between 68 and 72, and therefore the scene will run its actions. If the temperature is not in the range, it returns false, causing the scene to abort without running its actions.

The problem comes when allow is false (in this case, when the switch is off). The intent is that when allow is false, the return statement always returns false and therefore the scene actions never run. It’s supposed to be a master switch for the scene actions.

That’s the intent. But that’s not how it works. This is not a master switch. It is, in fact, an inverter. That is, when allow is false, it will cause the code to return false when the temperature is in the range, and true when outside the range–the inverse of its behavior when allow is true. It does not return false always. And the result of this is that the scene actions will still run, they’ll just run when you really don’t expect them to.

Why is this?

Let’s simplify the range condition part, which is ((tCurrent >= tLow) and (tCurrent <= tHigh)) down to the two possible result values it can have: true and false. It’s always true when the current temp is between the low and high inclusive, and always false otherwise.

Given that, then when allow is true–when the temperature is in range–the final test boils down to true == true, because the temp is in range (first true) and allow is true (second true). Therefore, the result of the expression is true. If the temperature is outside the range, the final test is false == true, which is, of course, false. So that’s fine.

Here’s where it goes wrong.

When allow is false, and the temperature is in range, the final test is true == false, which is false. But, when the temperature is outside the range, that final test becomes false == false which is true, and that true will be returned to the scene, which then happily runs the actions, even though allow is false.

Consider how potentially dangerous that could even be: the code will let the scene actions run when allow is false (the expectation that no scene actions run ever) and when the temperature is outside the range that would normally cause the scene actions to run. If it’s controlling a heater, for example, it could turn it on and leave it on long enough to start a fire, when the user’s expectation is that the code is not controlling the heater at all.

There are several ways to properly structure this code to get the desired result. One is simply:

if not allow then
    return false
end
return tCurrent >= tLow and tCurrent <= tHigh

The first block can also be written as one line if not allow then return false end, if you’re into the whole brevity thing. Either way, the separate, explicit allow test, done first, will cause the code to take the conditional branch and return false whenever and always if allow is false. If allow is true, then the returned value is the result of the temperature range test expression, whatever that might be.

The other way, if you prefer to write the return expression on one line, is something like this:

return allow ~= false and (tCurrent >= tLow and tCurrent <= tHigh) or false

If we follow from left to right, this abbreviated/boolean-ated code says "if allow is not false (i.e. true) then return the result of the temperature range test expression, otherwise return false). It’s a bit more confusing for relative newcomers to look at, but it’s succinct and it will work as intended. But if clarity is your preference, I would use the first example.

Like most memes, the “allow meme” needs to die, or at least, we need to understand how it’s wrong, and ignore it accordingly in favor of better options.

4 Likes

I ran the code in a lua editor for running scenes only between two dates and verified the issue with setting allow = false. Thanks for highlighting this.
I am not sure I follow your solutions though.

Allow is simply a constant correct? declared as true or false.
the ‘allow ~= false’ is resolves first as ‘true ~= false’ which is ‘true’ if allow is declared as ‘true’. or ‘false ~=false’ which is ‘false’ if allow is declared as false. so why use ‘allow’ at all? Just put in true or false.
Or if trying to keep clean code, just put ‘allow’ as it will be declared as true or false.

And I think this would only work if written as follows:

return allow ~= false == (tCurrent >= tLow and tCurrent <= tHigh) or false

The allow part could still be simplified, unless you are arguing to have allow only declared as true, in which case the ~= has to be used in multiple locations in the code. And, the second part of the line would be (assuming allow declared as true)

(True and ‘temperature expression’) or false

which resolves to ‘true or false’, hence ‘true’, if the temperature expression resolves to ‘true’. or ‘false’ if the temperature expression resolves as false.
So why not leave off the ‘… or false’ from the statement?
I searched around for examples of using the not operator. is this legal in vera lua? if so, it would work when you want to run the scene when outside the range.

return not ‘temperature expression’

I am also having a hard time with the if-then solution because the temperature expression is outside the if then statement (after the end). The if-then will always run and would return a true or false, in which case you could just ‘return true’ or ‘return false’ since allow, true, false are all booleans. I am not sure if ‘return’ ends execution of the code, but if it does, the temperature expression is never evaluated if allow is false. If retun does not end the program, then after the if-then runs the temperature expression runs and returns a true if in the range and false outside the range without regard to allow (which is just a true or a false anyway as stated earlier).

You don’t need allow at all. I put the allow in because the original script was using it to determine if the scene should run at all, like a master switch. The original code had a use for it, so I kept it in, but the point of my missive is to make that master switch value work as intended, which the original code did not. So it’s in.

No, that’s incorrect. You are now back to the same error that broke the original code. You’ve turned a master switch into an inverter, and that’s not the intent of allow. If the master switch (allow) is false, you want the return value to be false, period, always. By changing and to ==, you introduce the possibility that when allow is false and the tCurrent subexpression yields false as well, then false == false is true, allowing the scene to run when the master switch explicitly wants it off/not running. That’s the bug.

The use of and here uses shortcut expression evaluation, similar to C. If allow is false, then the expression allow ~= false is false, and nothing in the statement runs after that–the statement immediately returns false, and the tCurrent subexpression is not evaluated at all. If allow is true, then the expression allow ~= false is true, and the tCurrent expression is evaluated. The purpose of the or false at the end is to provide a catch-all so that any future expression placed withing the parenthesis is guaranteed to produce a hard false if it’s not true, which is required by Lua to prevent the run of the scene (Lua only accepts false, not nil or zero, etc.). It’s a safety to ensure a canonical return value.

This is exactly how you would want to write it if you weren’t using allow, but again, the point of the original missive was to show how allow’s intent could be preserved, but work properly, as all the previous examples were broken and didn’t function correctly.

That makes sense. In a long code with multiple locations that need an inverter or a master switch, ‘allow~=’ might be useful. In these shorter code segments, maybe ‘NOT’ is easier.

I think i see the confusion on the ‘and’ vs ‘==’ question and agree with you. I had originally interpreted ‘allow’ as an inverter.

I put the table below together to understand, and I think it shows the switch vs the inversion response. Both responses might be desired based on the case. In my case, I evaluate a date expression to true on a day within a date range. During that range, I want to inhibit the scene from running (invert to false). All other times of the year, I want the scene to run (invert to true).
The code should be clear about how ‘allow’ is used (as an inverter or a master switch).

image