Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Latest Action node seems to responding to payload values #1489

Closed
vertex-github opened this issue Aug 21, 2024 · 20 comments · Fixed by #1535
Closed

Latest Action node seems to responding to payload values #1489

vertex-github opened this issue Aug 21, 2024 · 20 comments · Fixed by #1535

Comments

@vertex-github
Copy link

Describe the bug

Prior to the latest release, Action Nodes just did their configured action, regardless of the msg.payload value. This worked great since I could trigger switch.on events/actions from a variety of flows, button presses, events etc. After the latest version, the Action nodes are rejecting all messages with a payload that they dont recognize - this is really unhelpful. eg. I have many 4 button panels that send different payloads depending on which button you click - button4-on, button2-off, button1-hold etc - previously all these events just got fed into the action node and the action node did its configured thing. Now all these flows are broken as the action node is rejecting the event payload. Can you please revert this change if it was intentional, or provide some original default with a new checkbox for the new behavior if that's really needed?

To Reproduce

  1. Have an action node configured to switch on a switch.
  2. Send in an event from a trigger node with payload = button1-press
  3. Note the error.
  4. Change the payload in (2) to be "on"
  5. Note that the switch is activated

Expected behavior

The action node should not care, by default, about the msg.payload of the message triggering the event.

Screenshots

No response

Example Flow

No response

Environment Information

Clicking that button pops up an empty window.

Node Red: 4.0.2
HA Version: 2024.8.2
node-red-contrib-home-assistant-websocket 0.67.2

Additional context

No response

@vertex-github
Copy link
Author

Logs from NR:
21 Aug 14:39:48 - [error] [api-call-service:Garage On] InputError: Invalid action format: on-press
21 Aug 14:39:52 - [error] [api-call-service:Garage On] InputError: Invalid action format: on-press
21 Aug 14:39:54 - [error] [api-call-service:Garage On] InputError: Invalid action format: on-press
21 Aug 14:40:37 - [error] [api-call-service:Garage Off] InputError: Invalid action format: off-press
21 Aug 14:40:38 - [error] [api-call-service:Garage On] InputError: Invalid action format: on-press
21 Aug 14:40:39 - [error] [api-call-service:Garage Off] InputError: Invalid action format: off-press

@zachowj
Copy link
Owner

zachowj commented Aug 22, 2024

Could you export and share the flow that demonstrates the error? I'm unable to reproduce it based on the steps listed in the 'To Reproduce' section.

@vertex-github
Copy link
Author

vertex-github commented Aug 22, 2024 via email

@zachowj
Copy link
Owner

zachowj commented Aug 22, 2024

The error indicates that the action field, now replacing the domain and service fields, expects a valid format like button.press. However, it's receiving an invalid format, such as off-press.

Are you using templates in the action field?

@vertex-github
Copy link
Author

vertex-github commented Aug 22, 2024 via email

@zachowj
Copy link
Owner

zachowj commented Aug 22, 2024

The action node (formerly the call-service node) has always supported config overrides via the msg.payload object. This should maintain backward compatibility with existing flows, except for the deprecation of the domain and service properties in favor of the action property, as noted in the release notes.

Could you provide an example flow that reproduces the error? This would help identify if something was overlooked.

@vertex-github
Copy link
Author

vertex-github commented Aug 23, 2024 via email

@vertex-github
Copy link
Author

Im also receiving the following errors from Github when posting here:

Remove 'waiting-for-response' label on issue comment by author / remove-label
Failed in 3 seconds

@vertex-github
Copy link
Author

vertex-github commented Aug 24, 2024

If you could add a checkbox toggle to the action node to not interpret any msg. Fields it would be helpful. Ideally default that disabled flag to true so we maintain compatibility with older flows that would resolve it. I've been using this setup of msgs as events triggering service call modes for 3+ years without issue so perhaps the old plugin silently ignored the unknown payloads and just triggered the call service node as configured?

@MostHated
Copy link

MostHated commented Aug 29, 2024

I am having an issue with this as well. I use Phillips Hue Dial devices which have a knob twist and 4 buttons. I have a function node which acts as a switch and uses some logic to determine light value step size then determines which output to route to:

Below is the code for this function node.

Function Code
var action = msg.payload.action;
var direction = msg.payload.action_direction;
var step = msg.payload.action_time;

if (action == null || action == undefined || action == "") {
  return;
}

var dialDirections = ["right", "left"];
var buttonNumbers = ["1", "2", "3", "4"];
var dialActions = ["step", "slow", "fast"];
var buttons = ["button_1", "button_2", "button_3", "button_4"];
var buttonActions = ["press", "press_release", "hold", "hold_release"];

// --| Set the desired button state in which to trigger the action
var desiredButtonTrigger = buttonActions[1];

var buttonStrings = {};
buttonNumbers.forEach(function (number) {
  buttonActions.forEach(function (action) {
    buttonStrings["button_" + number + "_" + action] =
      "button_" + number + "_" + action;
  });
});

var dialStrings = {};
dialDirections.forEach(function (direction) {
  dialActions.forEach(function (action) {
    dialStrings["dial_rotate_" + direction + "_" + action] =
      "dial_rotate_" + direction + "_" + action;
  });
});

// In the dial outputs, you can use either
// 'msg.payload.params.step_size' - to get a fixed step size per rotation increment based on the speed
// 'msg.payload.params.bump_value' - to use the speed of the rotation as the step size

// --| Example using Home Assistant Call Service node for a light, calling the 'light.turn_on' service
// --| Then in the 'Data' field, enter the following:
// --| { "brightness_step_pct": msg.payload.params.step_size,"transition": 0.5 }

if (direction == "left" || direction == "right") {
  var cmd = {
    command: direction == "left" ? "down" : "up",
    step_size: direction == "left" ? -1 : 1,
    bump_value: (direction == "left" ? -step : step) * 0.5,
  };

  if (action == dialStrings["dial_rotate_" + direction + "_step"]) {
    cmd.step_size *= 5;
  }

  if (action == dialStrings["dial_rotate_" + direction + "_slow"]) {
    cmd.step_size *= 10;
  }

  if (action == dialStrings["dial_rotate_" + direction + "_fast"]) {
    cmd.step_size *= 20;
  }

  msg.payload.params = cmd;

  return direction == "left"
    ? [msg, null, null, null, null, null]
    : [null, msg, null, null, null, null];
}

for (var i = 0; i < buttons.length; i++) {
  if (action == buttonStrings[buttons[i] + "_press_release"]) {
    var btncmd = {
      command: "release",
      button: i + 1,
    };

    msg.payload.params = btncmd;

    // --| Use the index determine the output in which the return message is sent
    // --| We use +2 because the dial actions are at index 0 and 1
    var returnArray = [null, null, null, null, null, null];
    returnArray[i + 2] = msg;
    return returnArray;
  }
}

return;

Here is the flow:

Here is the node:

As with the OP, my flow worked just fine prior, but now gives errors similar to "InputError: Invalid action format: dial_rotate_right_step". As far as I can tell, most other things work just fine, but items such as all of my dials, which only care about specific custom things in the payload, do not work anymore. I feel it is quite important for custom actions such as what myself and the OP are doing, to retain the prior functionality, whatever it happened to be.


Edit: Upon further investigation, it seems that the Phillips hue dial uses the "payload.action" for it's own existing purpose, so if I try to then clear it out, ex:

msg.payload.action = "";

I then get an error of :

"ValidationError: "action" is not allowed to be empty"

I don't think the action node should care about or validate the incoming action value because I am setting the action in the node itself.

If the incoming message has a payload property with action set it will override any config values if set.

I think that the above statement should be the opposite. I need to set the action myself, whatever was passed in, I do not care about.

@zachowj
Copy link
Owner

zachowj commented Aug 29, 2024

@MostHated msg.payload.action is now an input property of the action node, which was introduced as a breaking change.

When you set msg.payload.action = ""; it overrides the action field in the action node. This behavior is similar to how, in previous versions, setting msg.payload.domain = ""; would have triggered the same error.

@MostHated
Copy link

I just made an edit at the bottom.

Upon further investigation, it seems that the Phillips hue dial uses the "payload.action" for it's own existing purpose, so if I try to then clear it out, ex:

msg.payload.action = "";

I then get an error of :

"ValidationError: "action" is not allowed to be empty"

I don't think the action node should care about or validate the incoming action value because I am setting the action in the node itself.

If the incoming message has a payload property with action set it will override any config values if set.

I think that the above statement should be the opposite. I need to set the action myself, whatever was passed in, I do not care about.

@zachowj
Copy link
Owner

zachowj commented Aug 29, 2024

The intended use case for Home Assistant nodes has always been to allow setting default values in the node configuration, with the flexibility to override some or all of these defaults via input.

@MostHated
Copy link

MostHated commented Aug 29, 2024

Well, you can see the predicament here, I don't want or care about the action that was passed in. I want to use the action that I set in the node, but I can't remove the action that is being passed in.

This will probably end up being the case more often for a good while, too, now that the payload.action object is being used, as not only myself, but other devices were already using that name for other things, so making that override what is set in the config (which is the true intended action, which is why I set it) without the option to decide which behavior you would like to use, is going to break many things.

@zachowj
Copy link
Owner

zachowj commented Aug 29, 2024

@MostHated, In the action node update the data field to only include the default value: {"transitions": 0.5}. Then, pass any overrides through msg.payload.data in your function.

Alternatively, if you prefer not to modify the core of the function, you can store msg.payload.action in a local variable and then use delete msg.payload.action afterward.

@MostHated
Copy link

Ah, ok, thank you. That seems to have solved that issue. I didn't realize just deleting it was a viable option.

@vertex-github
Copy link
Author

vertex-github commented Sep 1, 2024 via email

@MostHated
Copy link

add a boolean checkbox that allows us to disable the interpretation of incoming messages and just do what the node is configured to do?

I definitely second this.

@ciaranj
Copy link

ciaranj commented Oct 1, 2024

Ugghhh, just ran head long into this, (hue switches), painful breaking change across all of my flows :( Can confirm that changing all of the action nodes to block overrides works.

Wanted to include the error I got so google can sign post everyone else here: "InputError: Invalid action format: on_press_release"

@Marcos-kp
Copy link

Marcos-kp commented Nov 5, 2024

I also would like to link the issue I am having: #1964
Only after updating the websocket I was able to get the debug messages and figure this out...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants