Global variables, hardware buttons and my pea-brain

I'm looking for help as I've spent far too long on a simple button problem in NR and clearly my head is now fogging up.

Here's the scenario - 2 buttons (on ESPs, grounded to operate - ESPs handle debouncing).

Status of the two buttons (0=pressed) comes into 2 similar functions in NR. All globals are initialised on NR power up.

Here's the function for button A (DOWN button) - both get "0" if button is pressed, "1" if released...

if (msg.payload=="1")
    { 
        global.set("buttonA","released"); 
        if (global.get("buttonB")=="pressed") { global.set("aWasPressed",0); msg.payload="a"; return(msg); }  
        else { if (global.get("bWasPressed")===0) { msg.payload="d"; global.set("aWasPressed",0);  return(msg); } }
    }
else { global.set("buttonA","pressed");  global.set("aWasPressed", 1); }

and button B (UP button) function (almost identical)

if (msg.payload=="1")
    {
        global.set("buttonB","released"); 
        if (global.get("buttonA")=="pressed") { msg.payload="a";  global.set("bWasPressed",0); return(msg); } 
        else { if (global.get("aWasPressed")===0) {  msg.payload="u"; global.set("bWasPressed",0); return(msg); } }
    }
else { global.set("buttonB","pressed"); global.set("bWasPressed",1); }

The buttons work a treat, returning 'd' and 'u' when pressed. If both are pressed at once, on releasing the first one I get 'a' for auto.. That should be the end of it, as "aWasPressed" and "bWasPressed" are there to trap the second release - but this doesnt work and the second button release returns it's normal 'd' for down or 'u' for up even after an 'a' for auto

I'm obviously missing something. Once cracked I'll blog it so all can share - right now I'm at my wits end. HELP.

Insert node.warn statements liberally in order to see what is going on.

I did - and it doesnt help. Thanks.

So which statement in the code is not working correctly? With node.warns you should be able to follow exactly what happens.

After I get an output of "a" for auto - ie both buttons were pressed.. I still get the "u" or "down". It should be prevented. I've edited the code to make it more readable - still not working though...
.

Remember you can put node.warn after the statements that set globals to check what value it has actually set.

I did. Didn't help, I must be doing something fundamentally wrong but I can't see it,

So if it is returning "u" then put node.warn in at that point to see if a global is not set right. Then put node.warn where it should get set to the right value and check that is happening.

After reading and thinking about this about an hour, I still can't figure out what is the main goal. Without understanding it completely, it is hard to figure out the problem.
Also I can't really understand purpose of bitwise operators usage.

When A is released and B is still pressed you set aWasPressed to 0 and send "a", then when B is released it sends "u" because aWasPressed is 0.

I think the logic would be simpler if you had an inAuto context variable rather than the wasPressed variables. However I think this may be an ideal candidate for a DSM solution and I would not be surprised if one came along shortly.

If I follow the code, it looks like the sequence below will produce what you're seeing.

A pressed   buttonA=pressed, aWasPressed=1
B pressed   buttonB=pressed, bWasPressed=1
B released  buttonB=released, bWasPressed=0 => a
A released  buttonA=released, aWasPressed=0 => d

The problem seems to be that when A is released, the code no longer knows B was pressed and released after A was initially pressed. Maybe you could try replacing aWasPressed and bWasPressed with buttonPressed having values of A or B or Both.

A pressed   buttonA=pressed, buttonPressed=A
B pressed   buttonB=pressed, buttonPressed=Both
B released  buttonB=released, buttonPressed=Both => a
A released  buttonA=released, buttonPressed=None => nothing

Under this model, you may no longer need to keep track of buttonA and buttonB.

A state diagram would probably help.

If I were going to do this in a function then rather than use two function nodes and global context I think I would use a Join node to join the two button inputs to a key/value pair then pass them to the function. In that function one then always has access to the current state of both buttons via msg.payload and the only thing that would need to be remembered would be the current Auto state on/off, which could be done with node context. I think that would end up with much simpler logic.

The goal is simple enough... I want to see if either up or down buttons are pressed, OR both buttons pressed. So that is 3 possible outcomes - "d" for down, "u" for "up", "a" for auto (for a heating system where adding a 3rd button would be painful).

"the "d" and "u" refer to heating states, NOT button states. Buttons sit logically high until pressed (grounded). I could not get my head around this until I introduced yet another global - and now it works.. but as Colin points out it needs the two functions and way too many globals - join node would be nice but I've no clue how to use that, Still - I have a result.

if (msg.payload=="1")
    { 
        global.set("buttonA","released"); 
        if (global.get("bWasPressed")==1)         { global.set("bWasPressed",0);if (global.get("lastCommand")!='a') { global.set("lastCommand",'a'); msg.payload="a"; return(msg); } }  
        else { if (global.get("bWasPressed")===0) { msg.payload="d"; global.set("aWasPressed",0);  global.set("lastCommand","d"); return(msg); } }
    }
else { global.set("buttonA","pressed");  global.set("aWasPressed", 1); }
if (msg.payload=="1")
    {
        global.set("buttonB","released"); 
        if (global.get("aWasPressed")==1) {  global.set("aWasPressed",0); if (global.get("lastCommand")!="a")  { global.set("lastCommand","a"); msg.payload="a"; return(msg); } } 
        else { if (global.get("aWasPressed")===0) {  msg.payload="u"; global.set("bWasPressed",0); global.set("lastCommand","u"); return(msg); } }
    }
else { global.set("buttonB","pressed"); global.set("bWasPressed",1); }

Or not depending on how familiar with state diagrams one is - I am not at all.

Example using Join node, it is really easy.
image

[{"id":"fdf5bfe6.5d0a1","type":"inject","z":"514a90a5.c7bae8","name":"","topic":"A","payload":"1","payloadType":"str","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":100.5,"y":443,"wires":[["a0cdb98f.65691"]]},{"id":"a2418b98.11965","type":"inject","z":"514a90a5.c7bae8","name":"","topic":"A","payload":"0","payloadType":"str","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":103,"y":483,"wires":[["a0cdb98f.65691"]]},{"id":"b6c8d40d.c36168","type":"inject","z":"514a90a5.c7bae8","name":"","topic":"B","payload":"1","payloadType":"str","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":101,"y":547,"wires":[["a0cdb98f.65691"]]},{"id":"71778657.6ee148","type":"inject","z":"514a90a5.c7bae8","name":"","topic":"B","payload":"0","payloadType":"str","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":103.5,"y":587,"wires":[["a0cdb98f.65691"]]},{"id":"a0cdb98f.65691","type":"join","z":"514a90a5.c7bae8","name":"","mode":"custom","build":"object","property":"payload","propertyType":"msg","key":"topic","joiner":"\\n","joinerType":"str","accumulate":true,"timeout":"","count":"2","reduceRight":false,"reduceExp":"","reduceInit":"","reduceInitType":"","reduceFixup":"","x":333.5,"y":525,"wires":[["65eb257a.cc636c"]]},{"id":"65eb257a.cc636c","type":"debug","z":"514a90a5.c7bae8","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","x":511,"y":565,"wires":[]},{"id":"ffb3d178.c40428","type":"comment","z":"514a90a5.c7bae8","name":"Function goes here","info":"","x":524.5,"y":522,"wires":[]}]

Wheeee. No need for a join OR global variables - I remember now how to use context vars and initialisee them. See code below - ONE function - no globals.

context.set("both",context.get("both") || 0);
context.set("ba",context.get("ba") || 0);
context.set("bb",context.get("bb") || 0);

if (msg.topic=="stat2/fromesp/trigger4") if (msg.payload=="0") context.set("ba",1);
if (msg.topic=="stat2/fromesp/trigger5") if (msg.payload=="0") context.set("bb",1);

if (context.get("ba")==1) context.set("both",context.get("both")|1);  
if (context.get("bb")==1)  context.set("both",context.get("both")|2);

if (msg.topic=="stat2/fromesp/trigger4") if (msg.payload=="1") context.set("ba",0);
if (msg.topic=="stat2/fromesp/trigger5") if (msg.payload=="1") context.set("bb",0);

if ((context.get("ba")===0) && (context.get("bb")===0)) 
    switch(context.get("both"))
    {
     case 1 :   msg.payload="d"; context.set("both",0); return msg;
     case 2 :   msg.payload="u"; context.set("both",0); return msg;
     case 3 :   msg.payload="a"; context.set("both",0); return msg;
    }

That is doing effectively what the Join node does, but you need to handle all the edge cases yourself, and find (or not) all the bugs due to forgetting to initialise things and so on. Given the choice between a Join and all that complicated code I prefer the Join, however that is just a matter of choice.

Thanks for that Colin - but there are 2 inputs not 4... producing outputs of "0" if pressed or "1" if released.

Output from the node should be "u" , "d" or "a" for up, down or auto. As you see I came up with a neat one-function, no globals solution - but I'm not proud, if you can vary your "join" version I'll give it a go. but by the look of it you ended up having to use a delay??? and cron job??

This is so useful on ESP826 where shortage of pins is always a problem, I'm so glad I persevered and the likes of yourself took an interest. The final answer will end up at https://tech.scargill.net with thanks to contributors.