Supergiovane / node-red-contrib-knx-ultimate

Control your KNX intallation via Node-Red! A bunch of KNX nodes, with integrated Philips HUE control and ETS group address importer.
https://youtu.be/egRbR_KwP9I
MIT License
143 stars 34 forks source link

Device node in universal mode doesn't update status when issuing read commands #305

Closed Mike-FUT closed 7 months ago

Mike-FUT commented 7 months ago

Hi Massimo,

When I use a normal device node, set the "Control" "Telegram type" to "Read" and I send any message to that node it will issue a bus read request and at the same time it will update the node status. When I do the same with a device node in universal mode and I send a read message to it (readstatus: true, destination: "1/2/3") it will issue a read request to the bus but it will not update the node status. It will simply stay at "Node Initialized". This might be by design or otherwise intended behavior but for me it seems strange, hence the report.

Steps to reproduce the behavior:

  1. Create a device node in universal mode
  2. Send a read request msg.readstatus: true, msg.destination: "1/2/3"

Expected behavior I would expect that status to be updated. But might be by design the way it behaves right now.

Knx-Ultimate Version 2.2.29

Are you running node-red behind homematic, docker or anything similar? No

Btw, this is zero priority, I just noticed it because I thought my program was buggy and I send data in the wrong format to the node (because it stayed at "Node Initialized" until I realized that it is actually sending data to the bus. So plain cosmetical issue.

Best Regards, Michael

Supergiovane commented 7 months ago

Hi Michael, Thank you for reporting this. It can be, that the node is not set to react to reponse telegrams. Have you set the node like this?

Node-RED___192_168_1_16
Mike-FUT commented 7 months ago

No, I set up the node like this: image

I basically want this specific one only to be used as a plain sender, nothing else. That's why I unchecked all three boxes.

Supergiovane commented 7 months ago

Set the option like mine. It will work without sending telegrams to the bus.

Mike-FUT commented 7 months ago

Ok, will try in a minute, but I don't want it to show any other telegrams in the status which it will do I guess...

Btw, this works, status is will be updated when issuing a read: image

Supergiovane commented 7 months ago

Why you don't want any status update other than that group address? You couldn't, currently, do what you want, sorry!

Mike-FUT commented 7 months ago

Why you don't want any status update other than that group address?

I love the node status for troubleshooting new code. If I display everything like in the suggested setting there are so many telegrams that's pretty hard to catch the right moment since it will not only show telegrams issued by messages from the preceding nodes.

Anyway, no big deal, thought since this is working for GroupValue_Response, for GroupValue_Write in universal node and it is working for sending, replying and reading in group address configured nodes I thought it's a mistake that it doesn't show read requests. Because the status is working correctly everywhere. Only not in this specific case.

Mike-FUT commented 7 months ago

Have you set the node like this?

Just tried... It still doesn't show the read request in the node status. Only the reply.

Supergiovane commented 7 months ago

The read status means nothing, there is no payload to show, but if you tick the “react to read requests” as well, you’ll see the status changing for the read request

Supergiovane commented 7 months ago

PS: you could also use the node “Viewer Node”, that uses the web UI to show you all the state. https://github.com/Supergiovane/node-red-contrib-knx-ultimate/wiki/knxUltimateViewer

Mike-FUT commented 7 months ago

“react to read requests”

That's great, thank you! And there's hardly any spam with read requests, so that's a nice workaround!

I'll check the source code to see if I can find the bug. I'll give it a 10% chance considering my JS knowledge but if I do find it I'll create a pull request.

Thanks a lot and have a great evening! And... Happy hollidays, merry christmas, Buon Natale!

Supergiovane commented 7 months ago

Grazie!! If you need some modification, just ask. Have a nice Christmas.

Mike-FUT commented 7 months ago

If you need some modification, just ask.

I thought I kind of did by creating this bug report.

But since you closed it here I guess that you feel different of what is a bug (for me something that works in the same way in 5 out of 6 cases but works different in 1 out of 6 cases for no obvious reason feels pretty damn close to what I would call a bug). And since you are doing an amazing job with this project I'd rather not argue much more about it. I mean without your project KNX and Node-RED would not be what they are now. Oh, that reminds me, it's been a while since I last donated something to this project... :-)

Btw, I found a fix and implemented it and looks like it's working but I'll keep monitoring it for a while and if still all is fine I'll publish a pull request in two weeks or so.

Supergiovane commented 7 months ago

Hi Thank you. I’ve worked hard in the previous month, to implement the HUE control, so I’m late with my “real” job and i need a pause. if you wish, you can also paste here the code you changed, so I can implement it. Merry Xmas