-
Notifications
You must be signed in to change notification settings - Fork 111
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
Update UART driver with specs, documentaion, and read/2 #1508
base: release-0.6
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works, great! even when doing hundreds of uart:read per second!
1cd4e1b
to
54b5195
Compare
I pushed changes that eliminate the use of any timer or task in the C code, and instead added a I believe this is much more efficient, and less resource intensive than creating a timer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this new version! Besides the little bug on timeout result, could you please fix potential concurrency issues?
Works great still. nitpick: Tried |
Yes. I see what you mean now, I thought your comment about concurrency was related to the previous implementation, but I see now that you were talking about pre-existing concurrency problems. |
Exactly. And issues could be amplified by the new code. |
54b5195
to
b1bc0b0
Compare
abfdc3d
to
6f19486
Compare
This should be ready for another look. I made more bug fixes that should be checked. |
6f19486
to
c7fd896
Compare
0cdd473
to
38b751a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to take advantage of this PR to change the if (xQueueRemove(...))
into a while
? It would make queue overflow less likely.
38b751a
to
f3886f3
Compare
f3886f3
to
5ac66d6
Compare
I @almost overlooked this. Great idea, I added that change. |
6baafa5
to
56fd43b
Compare
httpd_example cannot load index.html if you open UART a certain way using this PR. This issue does NOT occur in commit 4d38f90 of this PR. Find my modified http_example code here robinchew/atomvm_lib@033a226 which is essentially the following code which spawns a process that opens UART a certain way before spawn(fun() ->
Port = uart:open("UART2", [{rx, 16}, {tx, 17}]), % need to set rx and tx, and maybe UART2 as well
io:format("port ~p~n", [Port]),
end),
httpd:start(....) Trying to load index.html would result with: ~$ curl -v http://192.168.1.43:8080/index.html
* Trying 192.168.1.43:8080...
* connect to 192.168.1.43 port 8080 from 192.168.1.4 port 37574 failed: Connection refused
* Failed to connect to 192.168.1.43 port 8080 after 306 ms: Couldn't connect to server
* Closing connection
curl: (7) Failed to connect to 192.168.1.43 port 8080 after 306 ms: Couldn't connect to server |
I did really want to do this, but realized we can't without a lot more rewriting, or requiring the uart_driver sit behind a gen_server... as we have it now if there is no message to read immediately, the caller can drop into a receive and get the next message, at which point they would likely use a read to get the next incoming message. There is no way for the application to know that we are about to send another message from the queue after the first message is received. |
56fd43b
to
85e827a
Compare
I did add log errors for FIFO overflow, parity and frame errors, and a warning when the uart buffer is full. At least this way application developers will be aware when these events occur, and debugging communication problems may be easier knowing if the wrong parity is used. |
417f9a9
to
769f4a9
Compare
Thank you for testing and the follow up here. This was a mistake, I believe it should work much better now. |
769f4a9
to
a70b36b
Compare
Add type definitions for input parameters. Add spec for all exported functions. Add documentation for functions. Signed-off-by: Winford <[email protected]>
Fixes possible concurrency problems by using a mutex for access to `reader_process_pid` and `reader_ref_ticks` in `uart_data`. Avoid possible overflow of the queue by handling all mesages in the interrupt callback. Now cleans up and returns errors, with improved log messages when initalization fails with a badarg in the configuration parameters, rather than aborting. Fixes incorrect pin integers being cast to terms. Pin numbers are validated for the chip the VM is compiled for before use to mitigate VM crashes from bad input parameters. Signed-off-by: Winford <[email protected]>
Adds a new uart:read/2 that takes a timeout parameter for reads. If no data is received during the timeout period `timeout` is returned, and the listener is removed allowing for another read without getting the `{error, ealready}` error tuple. Closes atomvm#1446 Signed-off-by: Winford <[email protected]>
a70b36b
to
082eaee
Compare
Updates the UART driver to include function specs and documentation.
Fixes possible concurrency problems, and insufficient sized memory checks.
Now cleans up and returns errors when initializing with badarg parameters.
Adds new
uart:read/2
with a timeout parameter.Closes #1446
These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).
SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later