rscada / libmbus

Meter-bus library and utility programs
http://www.rscada.se/libmbus
BSD 3-Clause "New" or "Revised" License
217 stars 137 forks source link

added set-address #118

Closed Thomas131 closed 6 years ago

Thomas131 commented 7 years ago

I added the possibility to change the primary address of a device.

Sorry if I made something wrong ... this is my first pull-request into a foreign repo.

lategoodbye commented 7 years ago

Hi Thomas, thanks for your pull request, but i can't accept this change. libmbus is a library. We need a documented and working function in mbus/mbus-protocol.c or mbus/mbus-protocol-aux.c So your new binary should use this function. Btw try to avoid magic numbers and use defines instead.

Thomas131 commented 7 years ago

Thanks for the answer and sorry for my late answer!

Sadly I don't have a mbus-device at home to test new code and I don't have much time too. If someone wants to make it pushable, feel free to use my changes. Don't expect a new commit (at least not in the near future).

mpseybold commented 6 years ago

Thomas, thx for the file. I tried it and it is workling #100 .

Most of your code is handling the argument strings. After that its setting up the Mbus handle, like in the other programms in the ./bin/ folder.

The magic numbers in question are the two source lines 173 and 174

unsigned char buffer[3] = {0x01, 0x7A, atoi(n_addr_str)}; mbus_send_user_data_frame(handle, address, buffer, 3);

In the MBus Doku Section 6.4.2 "Writing Data to a Slave" ( http://www.m-bus.com/mbusdoc/md6.php ) the values DIF = 01 (hex) VIF = 7A(hex) specify a "Primary Address Record" together. DIF : Data Information Field VIF : Value Information Field

Hope that helps?

robertsLando commented 6 years ago

Any news about this functionality ?

lategoodbye commented 6 years ago

Unfortunately i can't see any pull request which addresses my comments.

robertsLando commented 6 years ago

What about take the pull request and fix it someway? I think this is would be a nice improvement for this library. I'm developing a node-red node that uses node-mbus that uses this library And this would be the only one feature missing.

lategoodbye commented 6 years ago

No, fixing things afterwards isn't the right approach. But in case this feature is really need i could prepare a pull request. It would be nice to get test results with real M-Bus devices.

robertsLando commented 6 years ago

With node-red node there will be lot more user that use this lib so lot more testing and improvments.

lategoodbye commented 6 years ago

I don't have the resources to implement it by myself, so any pull request providing node-red support is welcome.

mpseybold commented 6 years ago

I'll try to piece a pull request together in june.

Apollon77 commented 6 years ago

The node-red will use my nodejs module in general, so it's no real PR "to add node-red support". As soon as the functionality is in the libmbus I can use this also and make it available in my nodejs module

robertsLando commented 6 years ago

Sorry @lategoodbye, maybe I explained it wrong, I'm already working on node-red-node and you can find it here: https://github.com/robertsLando/node-red-contrib-m-bus I'm just asking you if it is possible to add the 'primary address set' functionality to your lib, this will allow @Apollon77, the owner of node-mbus lib that uses your libmbus for m-bus protocol, to implement that function on 'nodejs side' and me to add it to node-red node

3 separate projects wired togheter for a wider m-bus protocol support :smiley:

lategoodbye commented 6 years ago

@robertsLando @Apollon77 Could you please this branch https://github.com/lategoodbye/libmbus/tree/set-primary-address ? Please be aware it's only compile tested yet. I tried to make it fool-proof.

robertsLando commented 6 years ago

Today I'm at a conference so I can't test your branch sorry, tomorrow Maybe, anyway I need it implemented in @Apollon77 node-mbus library to test it on node-red. Need to ask to him first

Apollon77 commented 6 years ago

Will try tonight or latest on the weekend. I will also try to reproduce the message with a MBus software to verify ...

Apollon77 commented 6 years ago

@lategoodbye I had a quick look at https://github.com/lategoodbye/libmbus/commit/73d58a9f7dcf2e3877a51aa083bbbbd0952568f3 ... should work, I will incorporate it in my branch for now.

I also tried with an real device and the software I use. They send

68 06 06 68 73 FE 51 01 7A 02 3F 16

or

68 06 06 68 53 FE 51 01 7A 02 1E 16

And Device acklowledge the change by sending

5E back.

Will now implement it and test

lategoodbye commented 6 years ago

I created a pull request with my version #134

Apollon77 commented 6 years ago

Great, will use this as basis now to implement in my lib too :-)

robertsLando commented 6 years ago

:tada: @Apollon77 let me know when the update will be aviable in your library :smile:

Apollon77 commented 6 years ago

We should close this one because implemented now