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

Add optional reset-before-request-data #95

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

arska
Copy link
Contributor

@arska arska commented Feb 5, 2016

  1. refactor device reset/de-select
  2. add optional reset-before-request-data

This feature is needed to read Sontex Supercal 531 reliably

@arska arska force-pushed the feature-reset branch 4 times, most recently from 085d981 to 6a9956a Compare February 5, 2016 18:17
@arska
Copy link
Contributor Author

arska commented Feb 5, 2016

fixed both issues

@lategoodbye
Copy link
Collaborator

Sorry, but i don't think mbus_init_slave and mbus_init_slaves_str are a true benefit to the library. Why not using mbus_send_ping_frame() directly? It's an issue in mbus-serial-request-data-multi-reply.c so it should be fixed there.

Here is my suggestion add additional mbus_send_ping_frame with address = MBUS_ADDRESS_BROADCAST_NOREPLY add the end of mbus_init_slaves. I don't like new parameters for "special" devices. It's good if the tools work generally.

Does it fix the issue with Sontex Supercal?

@arska
Copy link
Contributor Author

arska commented Feb 8, 2016

I removed mbus_init_slave* completely, you are right they have too little functionality of their own.

Certain devices need to be reset, I don't like to side-effects of sending a broadcast reset to all devices. Other implementations also offer to send reset before request (e.g. Terminal-Systems MGW986).

@lategoodbye
Copy link
Collaborator

But we don't send a SND_NKE in case of a secondary address. (Yes, i know that a SND_NKE deselect the slave but nobody prevent us from resending a select again).

As a user i would expect the same reset behavior. Doesn't #90 appear in case of secondary address?

@lategoodbye
Copy link
Collaborator

Hi @arska, do you think #97 is related to this issue?

@delreich
Copy link

I don't see how this, in its current form, solves anything. You'd have
to be very careful (or lucky) to avoid duplicate frames.

Quoting "The M-Bus: A Documentation, Rev. 4.8" (page 28-29):

The master must always contain a "Next REQ_UD2-FCB-image bit"
and also a "Next SND_UD-FCB image bit" for each primary slave
address used by its application layer. After sending a
SND_NKE-request to a slave adress both these "Next FCB-image
bit" associated with this address contained in the request must
be set. Thus for each primary address the first REQ_UD2 or
SND_UD telegram after a SND_NKE contains a set FCB-Bit.

Note that after a memory loss (usually due to a power failure)
of these "Next FCB-image bits" the master is required to send a
SND_NKE to all affected addresses. All subsequent RSP_UD2-
telegrams must contain the "Next REQ_UD2- FCB-image bit" of the
appropriate primary address as a FCB. This "Next REQ_UD2 FCB-
image bit" is toggeled after an error free link layer RSP_UD
telegram has been received.

The default really, really should send resets, and then start with the
FCB-bit set (i.e. C=7B for the first REQ_UD2). This should apply to
all cases (multi-reply or not, primary and secondary addresses).

Currently, the only correctly working scenario is multi-reply with
secondary addresses. Secondary addressing is the only way to get a
proper reset, and multi-reply is the only way to get correct FCB-bit
handling (given proper reset).

If you want / need a way to skip the reset, you'll also need to specify
somehow what state you ended in previously. Supplying the number of
read frames might work. I can't think of a reason to do this though.

I don't see any reason to always send a reset to secondary address.
Secondary selection is essentially a reset in itself, from what I
understand.

Page 64:

A valid selection telegram will not only set the internal
selection bit but will also clear all [...] internal "Last
received FCB"-memory bit(s) associated with secondary addressing
via the pseudo primary address 253 ($FD).

Only way I can see that would allow continuing requests to a secondary
address without resetting, would be to explicitly use 253 for follow-up
requests. Again, though, I can't see a reason.

I would suggest that, instead of adding an option and all that, fixing
the reset (to use the correct address, and only for primary) and FCB-
handling (in the single-reply case) is the way to go.

cc @Spindel

@Spindel
Copy link

Spindel commented May 20, 2016

I don't have much to add there, @delreich. More options won't really fix the somewhat broken state it's in right now, and I've always been disappointed in seeing an "--unbreak-something" option for any software.

@delreich
Copy link

Looks like mbus/mbus-protocol-aux.c could do with some refactoring. There's several functions there that do very similar things in subtly different ways.

Quick examples:

  • mbus_sendrecv_request largely reimplements mbus_send_request_frame (only with working FCB-handling) when it should be calling (a fixed version of) it.
  • mbus_read_slave is almost, but not quite, a single-frame version of mbus_sendrecv_request, except it also does secondary address selection (and thus reset) and uses the broken mbus_send_request_frame.

Might be more, but 8pm on a Friday isn't really the time to be looking for them.

sjlongland added a commit to widesky/node-mbus that referenced this pull request Jun 8, 2023
The rationale for the `init_slaves` call seems undocumented (it was
added when the `mbus-{tcp,serial}-request-data-multi-reply.c` files were
added to `libmbus`).  If the slaves do not respond to the ping
operation, then it needlessly wastes double whatever the time-out value
is set to.

The ping immediately before communicating with a slave by primary
address seems to be a work-around to a specific model of M-Bus meter:
the Sontex Supercal531.

rscada/libmbus#95

This adds a boolean flag that permits enabling or disabling this
work-around as needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants