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 ChangeSlaveId api #22

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

Conversation

wumingyu12
Copy link

No description provided.

@nqv nqv self-requested a review July 20, 2017 05:06
@andig
Copy link
Contributor

andig commented Sep 19, 2018

+1 for this PR. I would prefer a different name though, maybe something like SetSlaveId or SetTargetSlave. With API available you could hide SlaveId as well which would be a breaking change.
It might also make sense to panic if SlaveId is not initialised with a valid value?

@andig andig mentioned this pull request Sep 19, 2018
@andig
Copy link
Contributor

andig commented Oct 4, 2018

I'd still greatly welcome this getting merged as it would get rid of ugly duck typing like this:

if handler, ok := q.handler.(*modbus.RTUClientHandler); ok {
	handler.SlaveId = deviceid
} else if handler, ok := q.handler.(*modbus.TCPClientHandler); ok {
	handler.SlaveId = deviceid
}

@andig
Copy link
Contributor

andig commented Oct 7, 2018

@wumingyu12 would you mind adding Timeout to the API in the same way (getter, setter)? I think its also used throughout the handlers and deserves the same api exposure?

kgritesh pushed a commit to pashi-corp/modbus that referenced this pull request Apr 30, 2021
Always reconnect if connection was closed
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.

2 participants