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

[Bug]: Service re-uses last received topic value #512

Open
lougreenwood opened this issue Dec 27, 2022 · 2 comments
Open

[Bug]: Service re-uses last received topic value #512

lougreenwood opened this issue Dec 27, 2022 · 2 comments
Assignees
Labels
bug 🐛 There is at least high chance that it is a bug!

Comments

@lougreenwood
Copy link

NRCHKB Plugin Version

1.5.0

Node JS Version

0.0.0

NPM Version

0.0.0

Node-RED Version

0.0.0

Operating System

Home Assistant

What happened?

(Sorry for not using real version numbers, I didn't have access to Node Red installation when I wrote the bug report, let me know if you need them).

When a switch service (with no topic defined in it's context, not sure if this is related or not) receives a msg with a topic value it will pass through the topic (✅ this is expected)

Subsequent interactions with this service via HomeKit (e.g Home app), send messages from the service node using the last received topic (❌ not expected, it's not documented that topic property on a msg also works as config).

How to reproduce?

  • Create a switch service node with no topic configured
  • Send it a msg with a topic
  • See the topic topic outputted by the service node
  • Interact with the switch in the Home app
  • See that the last sent message is used as the topic

Expected behavior:

I would expect topic to not be saved by the service and instead it should just be passed through by the service.

Additional comments?

No response

Relevant log output

No response

@lougreenwood lougreenwood added the bug 🐛 There is at least high chance that it is a bug! label Dec 27, 2022
@Shaquu
Copy link
Member

Shaquu commented Aug 15, 2023

@crxporter, indeed looks like we are storing topic and sending it later.

  1. Can you replicate?
  2. Is that a bug?

onInput
node.topic_in = msg.topic ? msg.topic : ''

onValueChange
const topic = node.config.topic ? node.config.topic : node.topic_in

@crxporter
Copy link
Member

Topic is always an interesting topic to discuss... see what I did there?

Anyway, what would the expected behavior be? I would expect:

For a pass-through message:
-if incoming message has no topic, the outgoing message would be (first choice) the config topic (if it exists), or (second choice) no topic
-if incoming message has a topic, the outgoing message would be the config topic (if it exists, overwrite the incoming topic) or the same as incoming message (if no config topic)

For messages originating in the node:
-messages should be the config topic if it exists
-if no config topic, I would expect outgoing messages to match whatever the most recent incoming topic was

This is the "normal" way for topic to work in nodered. I've seen other nodes use this convention. Based on the code, and bug report, I think this is what the node is doing?

I use topic a lot in my flows. I have the topic for everything homekit set in the node and haven't ever had issues.

In response to the bug report-
The config of the node is not being set by the incoming messages. Restarting nodered should wipe any previous "remembered" topic, unlike a config which will be remembered. It's just using the latest incoming topic if there's not any topic set by the config.

And now - does anyone have a reason it should work differently from how it is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 There is at least high chance that it is a bug!
Projects
None yet
Development

No branches or pull requests

3 participants