DISTORTEC / distortos

object-oriented C++ RTOS for microcontrollers
https://distortos.org/
Mozilla Public License 2.0
430 stars 66 forks source link

USART hardware flow control support #37

Closed HITMAnsOFT closed 4 years ago

HITMAnsOFT commented 7 years ago

I couldn't find how to enable hardware flow control (RTS/CTS) for USART2 on STM32F103. I need it to interface with a GSM modem. Is it supported by distortos?

FreddieChopin commented 7 years ago

Hello HITMAnsOFT!

Unfortunately at this moment there is no proper support for hardware flow control in distortos, but I guess that basic stuff can be enabled by just setting appropriate bits in USART registers and enabling proper pins. A dirty hack for the first one would be to just modify contents of USARTx->CR3 register AFTER starting SerialPort driver. I think that no functional change to the drivers is required to use hardware flow control (for example I see no reason to use CTS interrupt), so it should generally work right away.

If you want, I could try adding proper support for that. Just let me know (;

Regards, FCh

HITMAnsOFT commented 7 years ago

Thanks for your reply! From my very little experience with STM32, I think just setting the flags should be enough (STM32Cube seems to be doing just that), I'll try now. Is there a C++ wrapper for setting flags in the registers?

FreddieChopin commented 7 years ago

I'll try now. Is there a C++ wrapper for setting flags in the registers?

No, there is no such thing in the repository - no need to complicate simple things (and the assumption is that in the "application" code you rarely need to interact with registers directly). You can probably do it like this:

#include "distortos/chip/ChipUartLowLevel.hpp"
#include "distortos/chip/CMSIS-proxy.h"
#include "distortos/chip/uarts.hpp"

#include "distortos/devices/communication/SerialPort.hpp"

char readBuffer[256];
char writeBuffer[256];
distortos::devices::SerialPort serialPort2 {distortos::chip::usart2, readBuffer, sizeof(readBuffer), writeBuffer,
        sizeof(writeBuffer)};

int main()
{
    // configure pins - TX, RX, CTS, RTS
    serialPort2.open(115200, 8, distortos::devices::UartParity::none, false);
    USART2->CR3 |= USART_CR3_CTSE | USART_CR3_RTSE;

    // use serialPort2 object
}

Regards, FCh

HITMAnsOFT commented 7 years ago

That's what I did eventually! Unfortunately I have another problem with my on-board FTDI interface, that's why I cannot test this. The board I'm working with needs a pull-up on the UART1 RX pin, because it has a simple diode-based level shifter. I'm trying to configure the RX pin (pa10) as an input pin: distortos::chip::ChipInputPin rx1{distortos::chip::Pin::pa10, distortos::chip::PinPull::up}; after opening the serial port, but it still won't receive data.

FreddieChopin commented 7 years ago
  1. did you enable GPIOA in Kconfig (or directly in RCC registers in your code, before enabling this pin?

  2. I think you should try to set the pin to "alternate function", even if this makes no logical sense (you need to set it as "af output" when in reality it is an input - people at ST like to make things more complicated than necessary...). Try to do it like this:

distortos::chip::configurePin(distortos::chip::Pin::pa10, pushPull50MhzAlternateFunction, true);
// similar thing for TX, CTS, RTS

In case of STM32F1 and USART it should probably work as "input with pull-up" too, so the problem is probably not here. But you don't need to use ChipInputPin object anyway - configuring the pin with function is probably easier and won't use RAM for the unused object.

  1. Did you try sending anything first? If you have some non-standard hardware it would be probably easier to send something first - this way could quickly verify that the hardware works in one direction, before trying to set the other one.

Please post complete code so I could take a look and maybe verify it myself on a NUCLEO board.

HITMAnsOFT commented 7 years ago

OK, here's what I did:

  1. Yes, I enabled all 4 GPIO banks from Kconfig. I could blink all LEDs on the board.
  2. I'll try with configurePin, but it seems I have more serious problems here: I tried the code on another board (chinese so-called Blue Pill) with a normal 3.3v CP2012 adapter, and still cannot loop the data back. Here is my code:
#include "distortos/ThisThread.hpp"
#include <distortos/chip/ChipOutputPin.hpp>
#include <distortos/chip/ChipInputPin.hpp>
#include <distortos/chip/ChipUartLowLevel.hpp>
#include <distortos/chip/uarts.hpp>
#include <distortos/devices/communication/SerialPort.hpp>

const size_t bufSize{100};
char uart1ReadBuf[bufSize];
char uart1WriteBuf[bufSize];
distortos::devices::SerialPort serial1{distortos::chip::usart1, uart1ReadBuf, bufSize, uart1WriteBuf, bufSize};
char buf[bufSize];

int main()
{
    serial1.open(115200, 8, distortos::devices::UartParity::none, false);
    while (1)
    {
        auto r = serial1.read(buf, bufSize, 0);
        if (r.second) {
            serial1.write(buf, r.second);
        }
        //distortos::ThisThread::sleepFor(std::chrono::milliseconds{10});
    }
}
  1. I'll try both directions separately now. The hardware is definitely working, I tested it with a STM32Cube-based app, both the USB2UART part and modem on USART2 part.
HITMAnsOFT commented 7 years ago

I changed the main loop to this:

    while (1)
    {
        auto r = serial1.read(buf, bufSize, 1);
        if (r.second) {
            //serial1.write(buf, r.second);
            serial1.write("Hello world!", 12);
        }
        //distortos::ThisThread::sleepFor(std::chrono::milliseconds{10});
    }

and tried to debug it. On the board with a standard 3.3v UART adapter, the read() function reads the first 1-2 bytes correctly. The write() function returns success, but nothing shows up on the terminal.

HITMAnsOFT commented 7 years ago

OK, I finally made it all work. For any UART to work, regardless of pull-ups or pull-downs, the following is needed:

    serial1.open(115200, 8, distortos::devices::UartParity::none, false);
    distortos::chip::configurePin(distortos::chip::Pin::pa9, distortos::chip::PinConfiguration::pushPull50MhzAlternateFunction, true);

To enable pull-up on the RX pin, I added this:

    distortos::chip::configurePin(distortos::chip::Pin::pa10, distortos::chip::PinConfiguration::inputWithPullUpDown, true);

To enable RTS/CTS:

    USART2->CR3 |= (USART_CR3_RTSE | USART_CR3_CTSE);

So my working code for a UART bridge is:

#include "distortos/ThisThread.hpp"
#include <distortos/chip/ChipOutputPin.hpp>
#include <distortos/chip/ChipInputPin.hpp>
#include <distortos/chip/ChipUartLowLevel.hpp>
#include <distortos/chip/uarts.hpp>
#include <distortos/devices/communication/SerialPort.hpp>

const size_t bufSize{100};
char uart1ReadBuf[bufSize];
char uart1WriteBuf[bufSize];
distortos::devices::SerialPort serial1{distortos::chip::usart1, uart1ReadBuf, bufSize, uart1WriteBuf, bufSize};
char uart2ReadBuf[bufSize];
char uart2WriteBuf[bufSize];
distortos::devices::SerialPort serial2{distortos::chip::usart2, uart2ReadBuf, bufSize, uart2WriteBuf, bufSize};
char buf[bufSize];
// Onboard LEDs
distortos::chip::ChipOutputPin ds1{distortos::chip::Pin::pb6, false, {}, false, true};
distortos::chip::ChipOutputPin ds2{distortos::chip::Pin::pb5, false, {}, false, true};
distortos::chip::ChipOutputPin ds3{distortos::chip::Pin::pb4, false, {}, false, true};
distortos::chip::ChipOutputPin ds4{distortos::chip::Pin::pb3, false, {}, false, true};

int main()
{
    serial1.open(115200, 8, distortos::devices::UartParity::none, false);
    distortos::chip::configurePin(distortos::chip::Pin::pa10, distortos::chip::PinConfiguration::inputWithPullUpDown, true);
    distortos::chip::configurePin(distortos::chip::Pin::pa9, distortos::chip::PinConfiguration::pushPull50MhzAlternateFunction, true);
    serial2.open(115200, 8, distortos::devices::UartParity::none, false);
    distortos::chip::configurePin(distortos::chip::Pin::pa3, distortos::chip::PinConfiguration::inputWithPullUpDown, true);
    distortos::chip::configurePin(distortos::chip::Pin::pa2, distortos::chip::PinConfiguration::pushPull50MhzAlternateFunction, true);
    USART2->CR3 |= (USART_CR3_RTSE | USART_CR3_CTSE);
    while (1)
    {
        auto r1 = serial1.read(buf, bufSize, 0);
        ds2.set(r1.second);
        if (r1.second) {
            serial2.write(buf, r1.second);
        }
        auto r2 = serial2.read(buf, bufSize, 0);
        ds1.set(r2.second);
        if (r2.second) {
           serial1.write(buf, r2.second);
        }
        //distortos::ThisThread::sleepFor(std::chrono::milliseconds{10});
    }
}

Of course this is not optimal, because both directions are not handled in parallel, but it's a good starting point to check the operation of both ports. I think the TX pin configuration should be done automatically by the UART driver, so it is a bug. I can open a separate issue if you want. The RX pin configuration is non-standard and board-specific, therefore it is OK to do it manually. The hardware flow control option can be added to the open() function as a nice-to-have feature.

FreddieChopin commented 7 years ago

Of course this is not optimal, because both directions are not handled in parallel, but it's a good starting point to check the operation of both ports.

Just put the operations on the second port in another thread and you'll have the operations paralleled (; For example like that (note that default minSizeor 1 is used for SerialPort::read(), two threads would not work as expected with the 0 that you used in your code! also each thread has its own buffer):

#include "distortos/ThisThread.hpp"
#include <distortos/chip/ChipOutputPin.hpp>
#include <distortos/chip/ChipInputPin.hpp>
#include <distortos/chip/ChipUartLowLevel.hpp>
#include <distortos/chip/uarts.hpp>
#include <distortos/devices/communication/SerialPort.hpp>
#include "distortos/StaticThread.hpp"

const size_t bufSize{100};
char uart1ReadBuf[bufSize];
char uart1WriteBuf[bufSize];
distortos::devices::SerialPort serial1{distortos::chip::usart1, uart1ReadBuf, bufSize, uart1WriteBuf, bufSize};
char uart2ReadBuf[bufSize];
char uart2WriteBuf[bufSize];
distortos::devices::SerialPort serial2{distortos::chip::usart2, uart2ReadBuf, bufSize, uart2WriteBuf, bufSize};
// Onboard LEDs
distortos::chip::ChipOutputPin ds1{distortos::chip::Pin::pb6, false, {}, false, true};
distortos::chip::ChipOutputPin ds2{distortos::chip::Pin::pb5, false, {}, false, true};
distortos::chip::ChipOutputPin ds3{distortos::chip::Pin::pb4, false, {}, false, true};
distortos::chip::ChipOutputPin ds4{distortos::chip::Pin::pb3, false, {}, false, true};

int main()
{
    auto serial2Thread = distortos::makeAndStartStaticThread<1024>(1,
            []()
            {
                serial2.open(115200, 8, distortos::devices::UartParity::none, false);
                distortos::chip::configurePin(distortos::chip::Pin::pa3, distortos::chip::PinConfiguration::inputWithPullUpDown, true);
                distortos::chip::configurePin(distortos::chip::Pin::pa2, distortos::chip::PinConfiguration::pushPull50MhzAlternateFunction, true);
                USART2->CR3 |= (USART_CR3_RTSE | USART_CR3_CTSE);

                char buf[bufSize];

                while (1)
                {
                    auto r2 = serial2.read(buf, bufSize);
                    ds1.set(r2.second);
                    if (r2.second) {
                        serial1.write(buf, r2.second);
                    }
                }
            });

    serial1.open(115200, 8, distortos::devices::UartParity::none, false);
    distortos::chip::configurePin(distortos::chip::Pin::pa10, distortos::chip::PinConfiguration::inputWithPullUpDown, true);
    distortos::chip::configurePin(distortos::chip::Pin::pa9, distortos::chip::PinConfiguration::pushPull50MhzAlternateFunction, true);

    char buf[bufSize];

    while (1)
    {
        auto r1 = serial1.read(buf, bufSize);
        ds2.set(r1.second);
        if (r1.second) {
            serial2.write(buf, r1.second);
        }
    }
}

I think the TX pin configuration should be done automatically by the UART driver, so it is a bug. I can open a separate issue if you want.

It's not a bug. It is just a thing which is not yet decided and/or finalized. The problem is that maybe for STM32F1 you really have just a few (1 or 2) options of pinout for each USART or SPI or any other peripheral. But for a chip like STM32F4 there can easily be like a dozen combinations of pinout for a single USART - counting only RX and TX pins. Adding RTS and CTS can rise the number of combinations in an exponential fashion. Doing it the way you propose would be extremely hard to implement in Kconfig - the hidden code to deal with all the combinations, which would depend on exact version of chip and package, would be just enormous.

For now my plan is to add support for describing board configuration with devicetree format, which would then be used as an input to code generator based on Python + Jinja2. Generated board code would properly initialize and instantiate everything that is needed - just like this is now done for LEDs and buttons. Using .dts text files as input would mean easy editing and a possibility of implementing a graphical tool in the future. Think of that as a Cube MX - with text input (.dts) in the first stage and with graphical interface in the second stage. Implementing this idea would probably mean that Kconfig would be used only for non-hardware related configuration (build options, stack sizes, enabling/disabling of some RTOS features, ...).

Work on that was started in this issue - https://github.com/DISTORTEC/distortos/issues/25 , but currently all that is implemented is a Work In Progess.

Tu summarise - for now this is expected that you have to manually configure all pins which are used by USART / SPI. In future this will probably still be true, but the code will most likely be auto-generated. When I implement any feature in distortos I check many other similar projects (frameworks, other RTOSes, HALs, ...) - generally they either leave pin configuration entirely for the user or use massive amount of support code to provide the user with a set of valid choices.

I also realize that the root cause of the problem you had now is that there is absolutely no documentation or example showing how to use USART of SPI. It's also on my "to do" list - any collaboration in this regard is very welcome!

The hardware flow control option can be added to the open() function as a nice-to-have feature.

Sure - I've added that to my to-do list (this list seems to be getting extremely long <: ) and I'll try to implement that soon. But if you want, I'm obviously open to pull-requests (;

It seems that your first impressions with distortos may be far from perfect, but I hope that these first experiences were not so bad for you to loose interest (;

HITMAnsOFT commented 7 years ago

Thank you for your support! Distortos is quite cool a project. It bothers me that microcontroller manufacturers still insist on plain C SDKs and libraries, when it's been proven multiple times that most C++ features are really zero-overhead if used correctly, and yield much nicer code.

I myself also tried a threaded application, only I used two threads, one for each direction, and just put join()s in the main(). Yes, minSize=0 only makes sense in single-threaded case, to be able to handle both transfers, whichever comes first (kinda half-duplex mode). The static threads didn't work for me for some reason, they didn't start, and join() returned immediately, so I used dynamic threads. Maybe it has to do with stack sizes? Do all the static thread stacks have to fit in the main thread stack?

Regarding the pin configuration, I'm really new to the STM32 world and only used stm32f103 so far (I'm coming from much simpler AVR land :) ), so I didn't think of re-routable UART pins. It was strange to me that only the TX pin needed to be configured separately, that's why I thought it's a bug.

Wow, a tool like CubeMX generating C++ code would be a killer!

I'm currently trying to write a new higher-level UART driver that supports std::vector data, strings, readline() etc, with variable-size receive buffer. I just realized that that may not be a good idea because of limited RAM available, so I may ditch that. What happens if a UART driver causes OOM :) ? Also I'm thinking of making it iostream-compatible instead. The current SerialPort with its void*s is still somewhat low-level compared to the rest of the code. What do you think?

I'm still very shy to contribute commits to this project, because I don't grasp the level of genericity required to support different STM32 configurations, and I can't test with other STM32s (F0, F4 etc...).

FreddieChopin commented 7 years ago

It bothers me that microcontroller manufacturers still insist on plain C SDKs and libraries, when it's been proven multiple times that most C++ features are really zero-overhead if used correctly, and yield much nicer code.

To be honest I don't think we would like to look at their C++ code - its enough to witness the "exceptional" quality of their C libraries (;

The static threads didn't work for me for some reason, they didn't start, and join() returned immediately, so I used dynamic threads. Maybe it has to do with stack sizes? Do all the static thread stacks have to fit in the main thread stack?

The StaticThread object contains its stack (and everything else), so if you create it as a stack variable inside main(), then yes - the stack has to fit. However I would expect the application to crash in such scenario (; You can either increase size of main's stack or instantiate StaticThread objects as file-scope variables - then they will be placed in .data or .bss, not on any stack.

If you still have that code which exhibited such strange behaviour, maybe you could post it here so I could take a look?

It was strange to me that only the TX pin needed to be configured separately, that's why I thought it's a bug.

It is a feature of STM32F1 - default config for almost all pins is "input", and the pins which would also be "inputs" when used with peripherals (USART's RX, SPI's MISO in master mode, ...) will work fine with no additional configuration. This will probably not work so well for other families, like STM32F4, but maybe this is true there as well... Only the pins which are "outputs" (USART's TX, SPI's MOSI in master mode, ...) or pins which are bidirectional (I2C's pins, ...) have to be set to "alternate function".

I'm currently trying to write a new higher-level UART driver that supports std::vector data, strings, readline() etc, with variable-size receive buffer. I just realized that that may not be a good idea because of limited RAM available, so I may ditch that.

I'm also planning to implement a full integration of SerialPort with stdio (printf(), scanf() and so on), so this may also be interesting for you. "Real" C++ APIs for streams are a little bit to heavy for microcontrollers, but this is all a matter of RAM - if you have a lot, then why not (; The problem with things you mentioned like std::string and std::vector is that they all use dynamic allocation, which is not very real-time friendly, and has many negatives when used with limited amounts of RAM (like fragmentation). But I don't want to discourage you - it's all just a matter of the chip you are going to use. If it has >100kB of RAM, then I guess these issues won't be so important. For chips with as little as ~20kB of RAM this will probably not be such a good idea (;

What happens if a UART driver causes OOM :) ?

Well, if your code would use new then it will execute default "out-of-memory handler", which will call abort(), which will in turn halt the system. If you replace the "out-of-memory handler" with your own handler then something else will happen (;

http://en.cppreference.com/w/cpp/memory/new/set_new_handler

Generally the code in distortos does not use dynamic allocation at all, with following two exceptions (unless I forgot about something else [; ):

Also I'm thinking of making it iostream-compatible instead. The current SerialPort with its void*s is still somewhat low-level compared to the rest of the code. What do you think?

The problem with iostream is that it pulls in enormous amounts of code which are not needed at all - for example a few hudred kB of code is pulled in for locale handling... Just add #include <iostream> at the top of main.cpp and notice the new size of compiled binary... And this is all without actually using anything from that header... For a typical "test" configuration this single include adds ~200kB of code and ~6kB of RAM - quite a lot...

I'm still very shy to contribute commits to this project, because I don't grasp the level of genericity required to support different STM32 configurations, and I can't test with other STM32s (F0, F4 etc...).

Don't be afraid (; If you want to implement a low-level driver, then it should be targeting as many chip families as possible - you may have noticed that currently there are 5 chip families supported, but there are only 2 USART, 2 SPI and 2 GPIO drivers - this is because that's enough for all these families (; If you want to implement something new (for example I2C), then I guess for a start it would be enough to support only one family, but the whole thing must still fit with the general pattern (virtual interfaces, divided into low-level hardware part nad high-level application side, ...).

If you want to implement a higher-level driver, then it should not depend on any hardware details at all, only use low-level abstract interface.

If you have anything in mind which you could contribute just let me know - we can discuss it and I'll help you with that as much as I can.

Regards, FCh