Apollon77 / node-mbus

Nodejs mbus module
MIT License
21 stars 15 forks source link

Multi telegram support #22

Closed lvogt closed 5 years ago

lvogt commented 5 years ago

Use mbus_sendrecv_request which handles multi-telegram replies Should have no negative impact... ?

Apollon77 commented 5 years ago

Did not knew that method ... will need some days to check it

Will this also solve the limit of max 25 values per request?

lvogt commented 5 years ago

My test setup retrieves all 125 entries from an ABB electric energy meter.

Why the automated checks fail, I do not understand...

Apollon77 commented 5 years ago

I will check it. Good idea, solves one of my todos ;-)

Apollon77 commented 5 years ago

The tests do also semantic checks and not only compile. So it seems that the new function is doing different requests on the bus and so the tests are not triggered correctly.

So I think the Tests befestigt be adopted to the „new“ messages. Do you want to try it ;-)

Apollon77 commented 5 years ago

So, will now try to fix testing and then have this as 0.6.0 as "beta"-tag on npm for final tests. WIll comment here once released

Apollon77 commented 5 years ago

Please have a check of npm install node-mbus@experimental (it is the 0.6.0 with your changes and dependecy updates. Works for you?

lvogt commented 5 years ago

I had to use --unsafe-perm as usual, but seems to work fine :)

Also npm aborted when it found my ioBroker adapter without npm or git repository until I temporarily moved the new module to a different folder structure...

Apollon77 commented 5 years ago

Seems that something is problematic: https://github.com/Apollon77/node-mbus/issues/19#issuecomment-445754961 ... any idea?

Apollon77 commented 5 years ago

@lvogt I have iobroker.mbus as 1.1.1 on GitHub with 0.6.0 as dependency ... can you please retest there too?

Apollon77 commented 5 years ago

@lvogt BTW I found a difference: https://github.com/Apollon77/node-mbus/pull/22/files#diff-6eba7d29384832da31e5941e3fdb81d4L306 the try for fcb handling is out now .. but yea don't know if problematic Need to think

Apollon77 commented 5 years ago

And I found one missing lock-release ...

Apollon77 commented 5 years ago

0.6.1 on github

Apollon77 commented 5 years ago

@lvogt seems that really the one missing "remove lock" caused problems, but seems ok now ... SO please also check / ioBroker-mbus Github now reference to 0.6.1

lvogt commented 5 years ago

I just installed 0.6.1 using npm and everything seems okay. I will test to your mbus 1.1.1 adapter some time later, because I have some other changes in my local copy, which I have to combine first (a patch to update unchaged states like I did for the modbus adapter, and changend serialport module which lists symlinks for each actual tty device)

Will you keep working on the mbus adapter so I better wait a few days, or are you finished for now?

Apollon77 commented 5 years ago

I only planned to make this version the 1.0.0 soon, but I have no current topic after the small fix from monday. You are welcome to do a PR then we could integrate in 1.0.0, so also can wait ... depends how logn you need :-)