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

Release exclusive access to serial port on close #190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

labradon
Copy link

After ending program execution and closing the opened serial port, the serial port could not be opened again because it is busy. After some investigation, this could be tracked down to the exclusive access to the serial port which is being set in Open(...).

To fix this, the serial port must be released from exclusive access in Close():

        // Release the serial port from exclusive access.
        if (mExclusiveAccess)
        {
            // NOLINTNEXTLINE (cppcoreguidelines-pro-type-vararg)
            if(call_with_retry(ioctl, this->mFileDescriptor, TIOCNXCL) == -1)
            {
                err_msg += ", ";
                err_msg += std::strerror(errno);
            }
        }

As an additional configuration option, I've added the exclusive flag that allows to specify whether to require exclusive access to the serial port or not.

… feature: Allow configuration of exclusive access for serial port
@mcsauder
Copy link
Collaborator

mcsauder commented Oct 1, 2024

Hi @labradon , nice work. Apologies for the delay in review. I'll get a chance to run the unit tests on my hardware setup later this week.

@crayzeewulf , any concerns?

@crayzeewulf
Copy link
Owner

@crayzeewulf , any concerns?

(Thanks, @labradon) Looks good overall @mcsauder . I would like to run some tests this week before merging too. Please give me a couple of days and I will report back with my test results.

@mcsauder
Copy link
Collaborator

mcsauder commented Oct 8, 2024

Hi @crayzeewulf . Existing unit tests continue to pass, but they were not capturing this particular case on my hardware on the master branch either. The PR seems good to me, but I'd defer to your experience here.

@labradon , can you reproduce the failure with the unit tests, or is there a unit test we can add to catch this case?

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.

3 participants