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.

Sign up for GitHub

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

Jump to bottom

Serial backend fixes #1092

Merged
merged 6 commits into from
Dec 5, 2023
Merged

Serial backend fixes #1092

merged 6 commits into from
Dec 5, 2023

Conversation

pcercuei
Copy link
Contributor

@pcercuei pcercuei commented Dec 1, 2023

  • Make sure the serial backend will properly exit and not lock up when iio_context_destroy is called;
  • Handle clients sending the BINARY command in ASCII when IIOD is already in binary mode (which happens when a new client connects)
  • Fix iio_context_clone() in the compat layer, which should return -ENOSYS with the serial backend, as only one context can be created at the same time. This fixes iio-osc running on top of the compat layer when using the serial backend.

mhennerich
mhennerich approved these changes Dec 1, 2023
View reviewed changes
Copy link
Contributor

@mhennerich mhennerich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pcercuei pcercuei force-pushed the pcercuei/fix-serial branch 3 times, most recently from c8a007e to 2da3ddc Compare December 1, 2023 15:03
@pcercuei
serial: Use non-blocking reads in a polling loop
6c50e70 
Instead of doing blocking reads, which are not cancellable and therefore
cause deadlocks when the .shutdown callback is called, use a fast
polling loop with non-blocking reads.

This fixes the read thread not closing on shutdown.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
serial: Remove dangerous debug print
a07e10a 
It was using the return value as a specifier for the string length,
which is fine as long as the return value is positive, but is most
certainly not fine if it is a negative error code.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
serial: Implement .readbuf / .writebuf
b9073c7 
This allows Libiio >= v1.0 to talk with IIOD <= v0.25.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
iiod-responder: Handle BINARY command in binary mode
e7b33e2 
Unlike with the network and USB backends, IIOD is unable to detect when
a client disconnected. A new client would then send the BINARY command,
which would not be understood by IIOD if it already was in binary mode.

Use the convenient fact that the BINARY string is exactly the same size
as our command opcodes, and compare each opcode read with the
"BINARY\r\n" string. In case of a match, simply send a code 0 to tell
the client that we are in binary mode, and continue as usual.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
compat: Fix behaviour of iio_context_clone()
4292df5 
The .clone callback was only implemented by the local and network
backends in Libiio <= v0.25.

Emulate this behaviour by returning -ENOSYS if using a different
backend.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei pcercuei force-pushed the pcercuei/fix-serial branch from 2da3ddc to 4292df5 Compare December 1, 2023 15:15
@pcercuei pcercuei mentioned this pull request Dec 1, 2023
Merged
rgetz
rgetz reviewed Dec 1, 2023
View reviewed changes
serial.c
nanosleep(&ts, NULL);
#endif
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be in utils.c rather than here? (I guess it's a single use case?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utils.c is for the high-level API, we'd have to export it as a public symbol (as the serial backend can be a module) and I prefer to avoid that as much as possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense.

@pcercuei
task: Fix deadlock between thread and iio_task_stop()
40d5514 
It is possible that iio_task_stop() will be called even before the
iio_task_run() thread enters the while() loop, where it waits for tasks;
in that case, both threads would be waiting on the condition variable
without anyone signaling it.

Address that issue by signaling the condition variable at the beginning
of the for() loop, and not at the end.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
@pcercuei
Copy link
Contributor Author

pcercuei commented Dec 5, 2023

Added a fix for a lockup that only @buha could trigger.

buha
buha approved these changes Dec 5, 2023
View reviewed changes
Copy link

@buha buha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, the last commit does fix the issue of iio_info hanging after printing all the info

@pcercuei pcercuei merged commit d9cd027 into main Dec 5, 2023
24 checks passed
@pcercuei pcercuei deleted the pcercuei/fix-serial branch December 5, 2023 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgetz rgetz rgetz approved these changes

@buha buha buha approved these changes

@mhennerich mhennerich mhennerich approved these changes

@tfcollins tfcollins tfcollins approved these changes

@nunojsa nunojsa Awaiting requested review from nunojsa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pcercuei @tfcollins @rgetz @mhennerich @buha