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

Modify UART Class to Make Use of the txBuffer #304

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

delta-G
Copy link
Contributor

@delta-G delta-G commented May 6, 2024

I opened this to replace #303 because it had a bunch of files included from submodules. This should be cleaner. Sorry for the mess.

This pull request makes changes in Serial.h and Serial.cpp so that the UART class will make use of the TxBuffer. This greatly reduces the amount of time that Serial.print will block, especially for long strings. Currently the class makes no use of the 512 bytes it has reserved for a txBuffer.

I've changed the write functions so that they will load characters into the buffer and start a transmission if one is not already going.

Because the HAL takes a pointer to the data, I added a char variable to read into so the buffer can be advanced. I cannot attempt to send a larger portion of the buffer because the HAL expects contiguous memory and it will break in the case where the string to send rolls over the end of the ring buffer. I will submit another PR later to deal with that if I find a good way.

While it is possible to submit an entire string to the HAL at once, the string is submitted by pointer so it creates a possible race if there is some other code that changes the string before it gets fully printed. By utilizing the txBuffer, this problem is also alleviated.

I have tested this code in several configurations and it appears to work just dandy. Please let me know what you think.

@delta-G delta-G changed the title Use txBuffer Modify UART Class to Make Use of the txBuffer May 6, 2024
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels May 6, 2024
@delta-G
Copy link
Contributor Author

delta-G commented May 6, 2024

I pushed another commit that adds the availableForWrite function.

I also noticed that the original commit on this PR also fixes flush. The original code had a busy wait for txBuffer.available() which would always return false because the txBuffer wasn't being used. With the code in this PR flush should now work.

@paulvha
Copy link

paulvha commented May 25, 2024

I have tried this change because it makes complete sense and also because I am having issues with the Sparkfun RA6M5 Serial1 port. I have copied the changes as indicated and I had issue :

  1. txc in write() was not defined. It is defined in wrappercallback() as : static char txc; I moved that definition global to solve it.

It then compiled and worked without a problem.

FYI The problem was not solved on the RA6M5 with this change..the first character received is somehow 0x0 instead of the expected 0x7E. It is sent by the device as monitored and detected with an external tracer (Saleae). Also the sketch does work unmodified with a UNOR4 Wifi. Different issue

@delta-G
Copy link
Contributor Author

delta-G commented May 26, 2024

I have tried this change because it makes complete sense and also because I am having issues with the Sparkfun RA6M5 Serial1 port. I have copied the changes as indicated and I had issue :

  1. txc in write() was not defined. It is defined in wrappercallback() as : static char txc; I moved that definition global to solve it.

It then compiled and worked without a problem.

FYI The problem was not solved on the RA6M5 with this change..the first character received is somehow 0x0 instead of the expected 0x7E. It is sent by the device as monitored and detected with an external tracer (Saleae). Also the sketch does work unmodified with a UNOR4 Wifi. Different issue

txc is defined as a member variable of the UART class. I did this because a global would be shared between instances and I didn't want to introduce issues if two UARTs were working at the same time. There is another txc defined in the interrupt callback as a local variable, but it is only used in the context of the handler. That's because the handler doesn't belong to any one instance.

@paulvha
Copy link

paulvha commented May 26, 2024

Thanks. My error as I missed copying the txc definition from the header file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
3 participants