Skip to content

trace_connect is called twice #2669

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

Closed
erlend-aasland opened this issue May 19, 2025 · 1 comment · Fixed by #2670
Closed

trace_connect is called twice #2669

erlend-aasland opened this issue May 19, 2025 · 1 comment · Fixed by #2670

Comments

@erlend-aasland
Copy link
Contributor

OS: macOS
pymodbus version 3.9.2

I supply the following dummy hook to the trace_connect argument during ModbusTcpServer instantiation:

def print_connect(connected):
    print("CONNECT:", connected)

Observed behaviour

Upon client connection (and subsequent disconnection), I get the following debug print on my console:

(venv) $ python ./server.py
CONNECT: True
CONNECT: True
CONNECT: False

Apparently, the callback is invoked twice upon connect, and once upon disconnect.

Expected behaviour

I would expect only a single callback invocation upon connect:

(venv) $ python ./server.py
CONNECT: True
CONNECT: False

Debugging

A quick, narrowed down, Git grep reveals the following invocations:

$ git grep -n 'self\.trace_connect('
pymodbus/server/base.py:64:            self.trace_connect(True)
pymodbus/transaction/transaction.py:205:        self.trace_connect(True)
pymodbus/transaction/transaction.py:209:        self.trace_connect(False)

It seems to me the invocation in server/base.py is superfluous, and should possibly be removed (?). OTOH, changing the behaviour would be a breaking change. OTOH again, this API is not exactly very well documented.

The superfluous invocation was added by @janiversen in commit bebccce, PR #2549, issue #2546.

@janiversen
Copy link
Collaborator

Pull requests are welcome.

If you want the API better documented, feel free to submit a PR. It is currently documented at the same level as other API calls.

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 a pull request may close this issue.

2 participants