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

Serial write code slow, blocks on IO, does not use FIFO buffer in SAMD21g #2

Open
cheetor5923 opened this issue Aug 6, 2019 · 11 comments

Comments

@cheetor5923
Copy link

Thread on Arduino Forums - https://forum.arduino.cc/index.php?topic=630048.0

Hi All.
I have an industrial robot which has a control system based on FreeRTOS. One task has the duty
of talking Modbus RTU over one of the USARTs to an IO expander at a rather slow rate of 57600..
the problem is the way the board code handles writes to the USART.

SERCOM.cpp from https://github.com/Industruino/IndustruinoSAMD/blob/master/cores/industruino/SERCOM.cpp


int SERCOM::writeDataUART(uint8_t data)
{
  // Wait for data register to be empty
  while(!isDataRegisterEmptyUART());

  //Put data into DATA register
  sercom->USART.DATA.reg = (uint16_t)data;
  return 1;
}

This effectively blocks the transmitting task until the TX is complete, Makes no use of the SAMD21G's 16 byte FIFO buffer or the stream buffer for Serial.write() (at least I think it does). I end up having to wrap all my Serial.write() in a vTaskSuspendScheduler() to ensure the timing doesn't get screwed up. This wastes a massive load of processor cycles I could be using for other tasks. I don't like having some of the more critical threads blocking for sometimes more than 50ms.

Ideally I'd like to make use of the FIFO buffer offered in hardware, use an ISR to handle filling the hardware FIFO from a larger software one, meaning I don't need to wrap my modbus writes with a
scheduler disable, as the ISR can easily keep the timing good on a 48MHz RISC core.

IIRC Serial.write() is supposed to be non-blocking?...
What do you think? some of this core hardware level stuff is a bit beyond me. It strikes me that I could
copy/paste some code from the Arduino Due implementation since the hardware is very similar?. I've had a look at the Due UART code and it looks quite different.

@tomtobback
Copy link

the IndustruinoSAMx core seems to use a newer version of the ArduinoCore-samd so i assume you could use this version of SERCOM.cpp https://github.com/Industruino/IndustruinoSAMx/blob/industruino/cores/industruino/SERCOM.cpp

@ta2edchimp
Copy link

Hi @tomtobback

sorry to hijack this topic, but is there anything that prevents releasing a new version of SAMD21G board definitions, based on the Industruino/IndustruinoSAMx repo?
Atm I use a local copy of the latter for the SAMD21G board (local fork of that repo, replacing the official 1.0.1 on disk), but I had to also install the officially released SAML21B definitions via Board Manager for the tooling paths to properly work within the Arduino IDE. Thx

@cheetor5923
Copy link
Author

Thanks Tom. That newer code looks FAR better.. going to test it out on my next build. Will let you know how it goes.

@cheetor5923
Copy link
Author

Update. install instructions for the SAMx core are broken. They install this library instead.
Tried manually clearing out the 1.0.1. folder and sneaking in the SAMx packages with no luck either.

@tomtobback
Copy link

tomtobback commented Aug 13, 2019

many thanks @ta2edchimp for your feedback, i agree we should work on a release of 1.0.2 SAMD board definitions based on the SAMx repo
i'm not sure how long this will take so in the mean time @cheetor5923 could you give it a try the way @ta2edchimp mentions above, replacing the 1.0.1 files with the SAMx version (as you already did?) and then also installing the SAML package in the normal way via the board manager as described here https://github.com/Industruino/IndustruinoSAMx#installation-on-arduino-ide (which will add the 4-20mAker board)

@cheetor5923
Copy link
Author

Hi Tom, Thanks.

The instructions listed on the SAMX page are flawed. for the SAM21D, they install the 1.0.1 version not the SAMx one. I'm not concerned with the SAML since I'm not using one. I've tried manually installing it, but that's led to even more trouble and bugs in the Industruino codebase(SAMD and SAMx alike).

also I'm trying to get this to work in PlatformIO on VSCode(which lacks a board def) I've tried to make one, but really its a tad beyond me - platformio/platform-atmelsam#27

@ta2edchimp
Copy link

ta2edchimp commented Aug 13, 2019

Hi @cheetor5923

please try the following:

  • install the 1.0.1 version of the SAMD package as instructed

  • also add the SAML package by adding the second URL to the Arduino Board Manager
    (this one is required for the correct tooling in the following although you're not using the SAML board itself)

  • go to the location where the Arduino packages are located on your disk
    and within the industruino/samd/ directory, simply remove the 1.0.1 folder (or make a backup somewhere else)

  • clone the SAMx git repo into a new 1.0.1 folder
    (its content must align with this repo's root, so there have to be the boards and platform text files)

  • make sure you checked out the industruino branch

  • execute the following shell script and parameter set-arch.sh samd
    (if you are on Windows, use WSL or Git Bash or sth. like that; alternatively, the boards, platform and programmers text files have to be renamed and concatenated according to the corresponding shell scripts, drop me a line if you have trouble while doing so)

Using these steps, I was instantaneously able to successfully compile and run my sketches.


As a side note: I also tried to get this to work in PlatformIO a whole while ago and while it worked good for the old 1286 board, I had (wrt to the linked pio issue) exactly the same problem with the modified bossac tool.
If I see this correctly, the new SAMD/SAML packages come with a bossac variant that looks like the unmodified one, but with a tweaked configuration to upload the binaries to that higher starting address. @tomtobback can you confirm this observation?

@tomtobback
Copy link

thanks @ta2edchimp; i can't confirm the bossac details for now but the new version for SAMD is in the pipeline for September

@cheetor5923
Copy link
Author

Hi all. have managed to get the SAMx brance into platformio without any problems (except for uploading over usb) compiling, uploading and debugging work perfectly (over Atmel ICE). I wrote a board def .json for the Industruino.
industruino_d21g.json.txt

@cheetor5923
Copy link
Author

cheetor5923 commented Aug 14, 2019

Fix one bug, find another.... opening the USB VCP causes I2C to break.. lovley
Edit: Turns out I broke it... Silly me, even have an entry in my changelog from may "Check Indio library for thread safety" which I never did...

@limitedAtonement
Copy link

thanks ta2edchimp; i can't confirm the bossac details for now but the new version for SAMD is in the pipeline for September

@tomtobback Arduino just told me that Industruino SAMD version 1.0.2 is available. Is that the before-mentioned new version?

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

No branches or pull requests

4 participants