Apollon77 / node-mbus

Nodejs mbus module
MIT License
21 stars 15 forks source link

Working on node-red m-bus node #5

Closed robertsLando closed 6 years ago

robertsLando commented 6 years ago

I'm working on a node-red node that uses your package, I will keep you updated ;)

https://github.com/robertsLando/node-red-contrib-m-bus

Apollon77 commented 6 years ago

cool! Write on any questions or topics

robertsLando commented 6 years ago

Sure, will keep you updated. Does your library has events listeners implemented? I would need events on m-bus clients connection like "onError" "onConnect" "onClose"

Apollon77 commented 6 years ago

Nope, the underlaying library (libmbus) do not provide a conneted info at all, so it is really "by request" where you see error or not. :-( I thought about this to implement that on the node-side but it is intransparent which error would have what meaning ... when we have enough examples I could do that to try to detect a "disconnect" by the returned error messages ... but till now it would only be a "wild guess"

For "onConnect" you can use the callback in the "connect" method.

For "onError" you should react on the "error" parameter in the callback from getData because it can also be that only one device on the bus got disconnected ... this is the problem with "multiple device bus-ses"

robertsLando commented 6 years ago

Ok and what about a prototype function isConnected that returns the status of connection at least?

Apollon77 commented 6 years ago

PS: I think the important understand topic is that "connect" and "close" ohne mean the "connection to the MBus-Master" (USB stick or TCP Master-device) and NOT "if a slave is available on the bus or not". Means:

Apollon77 commented 6 years ago

The problem with "isConnected" is exactly descibed above: When I connect to the Master then I can set "connected=true" and when I disconnct I set it to "false" ... but there is no way to really make sure these values are correct because the connection could be crashed or such and I would not know that - beside that each "getData" call will return some kind of error ... and so we are on "how to see out or errors that the "underlaying master connection" is broken and not "this one device is not responding".

So I decided for now to not have exposed a "connected" status because I can not make sure it is correct :-(

I also implemented the library in a plugin and I do there the following: I know how many devices are queried and as soon as I got errors for all of them at least once then I assume that the master connection got broken and close/reconnect and so "start over" ...

robertsLando commented 6 years ago

Could you provide me the code you have used to manage this please? So I can implement the connection mqnagement in the node too

Apollon77 commented 6 years ago

Don't know if you really can reuse code, but it is implemented here: https://github.com/Apollon77/ioBroker.mbus/blob/master/main.js#L134

So I use a array to store an error flag per device ID queried. As soon as length of the Array matches length of the list of device IDs at all then i call (in that "onClose" a close and restart completely, what calls connect again

Apollon77 commented 6 years ago

PS: One other way could be to use connect->getData->close on ANY query of devices because "connect" will false too if unsuccessfull ... then you could know that it is the Master connection ...

robertsLando commented 6 years ago

I think I will go for you last option for the first implementation, I will open and close connection on every request

Apollon77 commented 6 years ago

But then you need to make sure to "serialize" these ...

robertsLando commented 6 years ago

I have decided to create a client and a reader, the reader ATM just subscribes to client updates and sends messages to output on scan complete and on every device update.

Once the client is inited it scans for secondary using scanSecondary function than it start a function that keep reading all devices (one by one) and uses the same logic you showed me to check for errors and restart connection if needed.

The first problem I'm expecting is that the function scanSecondary never returns (no error no data returned it seems stucked)

Could you take a look to my code and tell me if you have any suggestion?

Apollon77 commented 6 years ago

How long you wait? The minimum needed time is around 80s for a scan, it can be multiples of this when more then one device on the bus is within the same "secondary ID range" because then scan get's deeper.

When you install the package using "--debug" the native part is compiled with debug output. For a normal process this goes to stdout ... maybe try this and you see what is send and received in the native part.

robertsLando commented 6 years ago

Is there a way to set the parity even/odd on serial connection?

Apollon77 commented 6 years ago

mbus is using "parity even" by definition and should be set automatically (already). Is this your problem?

robertsLando commented 6 years ago

Nope sorry I thought the problem was the parity I didn't know parity even is a standard for m-bus. Anyway first working version is on npm, still some bugs and fix needed but It works

Apollon77 commented 6 years ago

And was it the length of the scan that confused you or an other error?!

robertsLando commented 6 years ago

Nope, I still don't know why the scan doesn't timeout when the connection established but nothing is read, for example I was unable to read because I had set a wrong baud, the scan never returned and stucks until I manually disconnect usb. Even if I disconnect client seems something goes wrong because node-red doesn't exit I need to close it forced. I think there is something to check on connection management...

Apollon77 commented 6 years ago

Hm, interesting. What kind of OS you have? Did you tried the "debug" install with additional logging? THis would be interesting to see. I successfully tested it on WIndows and Linux by myself and I have setup testing on Linux, MacOS and Windows on CI

robertsLando commented 6 years ago

Didn't tried the install with --debug option, will try that tomorrow as I'm not on my pc right now, I also need to create a Readme and the help section of nodes, many things to add tomorrow :) if you want to give a try to the node or clone the repo I would be glad for the support :)

Apollon77 commented 6 years ago

Don't have node-red in use at the moment ... I'm bad reachable tomorrow because of a company meeting ... will try maybe on weekend

robertsLando commented 6 years ago

Sure no problem, tomorrow I will make my best to make it work, will keep you updated! :)

robertsLando commented 6 years ago

Still same problem, I have installed your package with --debug option and the scan stucks here:

mbus_serial_send_frame: Dumping M-Bus frame [5 bytes]: 10 40 FD 3D 16 
mbus_serial_recv_frame: Attempt to read 1 bytes [len = 0]
mbus_serial_recv_frame: Got 0 byte [remaining 1, len 0]

mbus_serial_send_frame: Dumping M-Bus frame [5 bytes]: 10 40 FD 3D 16 
mbus_serial_recv_frame: Attempt to read 1 bytes [len = 0]
--------^C (here I stop Node Red with Ctrl+C)----------------
16 Mar 09:24:25 - [info] Stopping flows
16 Mar 09:24:25 - [warn] [mbus-client:test] Connection closed
----Node red remains stucked and don't stops ----

This only happens if I start node-red with usb connected. If I connect usb after node-red start everything works fine

robertsLando commented 6 years ago

Have you ever tried to disconnect the USB or simply close the client connection while the scan or a data request i running? I'm expecting segmentation fault (core dump) error, other times it happen that the application doesn't exit as showed in the previous comment. There are some connection bug here to fix, Is there any way to prevent this? I think there should be a try catch somewhere to catch that error when a request fails

Apollon77 commented 6 years ago

to the Log: very interesting because I expected blocking issues at all, but here he send out data, run into timeout (normal behaviour) and send second time and should end in timeout again ... would all be normal. timeout is around 3s currently. Also your info that you have these problems when it was plugged in before vs after. Should all be irrelevant. So question is if you run it with the example.js alone (so no node-red!!) ... behave it the same? Or different then? What system you use it on? Maybe it is a driver issue? If Linux what's the output of "dmesg" after you plugged it in? Do you have a second system to try to reproduce it?

I also tried USB disconnects sometime ... can retry on the weekend. But also here: never had such problems ... In my tests such a case is in too in one of my projects and it worked well. Will ad more dedicated testcase for this later on.

robertsLando commented 6 years ago

Thanks for your support @Apollon77, I will make more tests on Monday. I have fixed some errors today and I have provided some documentation on Readme and node-help section. If it is possibile for you to check my code and tell me if I do something wrong it would be great. Anyway someway will make it work ;)

robertsLando commented 6 years ago

Anyway I think the problem related to node-red start with usb connected it's something more related to node-red, I think it's a thread related problem, I think that problem could be fixed someway with other solutions maybe a timeout or something else, will try more on Monday

Apollon77 commented 6 years ago

Will have a look on your code tomorrow

robertsLando commented 6 years ago

I have made some test this morning and there are no errors thrown by using example.js and even the scan doesn't stuck, I don't understand what is going on when using node-red, I think is something node-red related. Will keep you updated

Apollon77 commented 6 years ago

Great to hear on one hand ... not-understandable on the other :-(

I also started looing int your code and hopefully continue tonight ... time is rare at the moment :-( Also will check your PR. Thank you !

robertsLando commented 6 years ago

Finally fixed! There are still some strange bug but now works! 🎉

Apollon77 commented 6 years ago

What was it?

robertsLando commented 6 years ago

Sincerly I don't know, I still think was something node-red related, just used restartConnection method instead of connect to start the connection (I don't mean node-mbus methods but mbus-client.js methods)

robertsLando commented 6 years ago

Hi @Apollon77 ! Two important questions:

  1. What about promises? I could try to make a PR to make node-mbus methods works with promises.
  2. Is there any function provided from libmbus to set the primary ID of a device? I think it is the only functionality that is missing in this package :)
Apollon77 commented 6 years ago

to 1.) If you can provide an PR for this so that it can work with callback OR Promises then I would be happy. The only question is which Promises to use? I would like to go with the "official" nodejs style and no third party lib. Maybe a second issue/the PR to discuss it further :-)

to 2.) not till now. There is an open/wip PR on libmbus to add that (https://github.com/rscada/libmbus/pull/118 ) As soon as this is approved or at least "good enough" I can use this as basis to add this functionality to the node-lib ...

Apollon77 commented 6 years ago

Fixed and also setPrimaryId is in 0.5.0 available