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

Callback for Serial Class #160

Closed
vChavezB opened this issue Mar 1, 2022 · 11 comments
Closed

Callback for Serial Class #160

vChavezB opened this issue Mar 1, 2022 · 11 comments

Comments

@vChavezB
Copy link

vChavezB commented Mar 1, 2022

Overview

I have been working on a project that requires processing the reception of UART frames in microseconds. The current API for the Serial class does not have a way to include user callbacks for whenever UART frames are received.

I would like to request a feature to add a callback function for the reception of frames as this gives the user more control over the data for real-time scenarios. I know the Arduino SDK provides basic APIs to access the HW peripherals but this would enrich the framework by providing advanced API methods for real-time scenarios.

As an example Espressif has an API to register callbacks for the UART peripherals of the ESP32. So I do not think I am the first to think this feature is useful.

Context

I work with Master's students at a University and giving them advanced API features would allow for more control of the Hardware peripherals while using the Arduino SDK.

Proposal

I have implemented already this callback API for the Arduino Sam Core in a fork as I needed the feature:

https://github.com/vChavezB/ArduinoCore-sam

I also made a pull-request and discussed this in a similar pull-request from another user.

As you can see, there is interest for advanced API methods in the SDK but unfortunately (at least) in the Arduino SAM repo there is no activity from the maintainers which could enhance the current status.

Open-questions

If this feature is of interest to be integrated by the Arduino Development Team, the main open-points are :

  • What will happen to the circular buffer if the user attaches its own callback? Should it continue storing the RX frames?
  • What type of callback is required? For my forked, I added the option of adding a callback parameter. This is useful for scenarios where you are using OOP and passing around object instances or if using an RTOS passing the context.
@matthijskooijman
Copy link
Collaborator

I would welcome such a callback, I also have a need in another project (with another core, but having a standardized official API for this would be good).

What will happen to the circular buffer if the user attaches its own callback? Should it continue storing the RX frames?

I can imagine it works as normal, and the user should just call available() and/or read() from the callback to process the data. This has a bit more overhead, but probably results in the cleanest code (and allows the callback to decide to handle something or leave it in the buffer).

What type of callback is required? For my forked, I added the option of adding a callback parameter. This is useful for scenarios where you are using OOP and passing around object instances or if using an RTOS passing the context.

A function pointer with a void* extra argument would be good to do from the start, since it allows doing all kinds of things on top of that. In some APIs, I think there is also support for passing a std::function (which uses a bit of dynamic memory, but easily allows things like lambdas), though that cannot be ported to AVR (but could be added as an optional feature).

@PaulStoffregen
Copy link

Originally serialEvent was to be interrupt based callback. But during initial testing, every example program created was not interrupt safe, especially those using String. Ultimately it was decided interrupt context callbacks should not be part of the Arduino API for data processing like Serial. Maybe it is time to revisit and change that decision. I am only writing this to give some historical perspective about the prior decisions made many years ago.

I'm also curious whether std::function works on AVR? Last time I asked (admittedly a couple years ago) the strong opinion was it could not be considered for Arduino API until avr-libc improved. I'm also wondering under which circumstances does std::function call new / malloc() on supported platforms?

@matthijskooijman
Copy link
Collaborator

Originally serialEvent was to be interrupt based callback. But during initial testing, every example program created was not interrupt safe, especially those using String. Ultimately it was decided interrupt context callbacks should not be part of the Arduino API for data processing like Serial. Maybe it is time to revisit and change that decision. I am only writing this to give some historical perspective about the prior decisions made many years ago.

I believe this would be good to revisit, even if only in an "advanced" API intended for more advanced users and/or libraries. This does need good documentation, though, and also requires that other code (like Serial itself) is properly written to be usable from ISRs (this is done on AVR, I think still pending for SAM/SAMD).

I'm also curious whether std::function works on AVR? Last time I asked (admittedly a couple years ago) the strong opinion was it could not be considered for Arduino API until avr-libc improved.

Basically it is just not there, on AVR libstdc++ is not enabled, only libc is. There are some filler libraries that use uclibc++ to fill this gap, but that (at least upstream) does not have the C++11 support to (IIRC) using lambdas with std::function, limiting its usability.

I suspect that it might actually be possible to just enable libstdc++ for AVR (similar to how it is on ARM), I remember seeing some postings over the years of people trying this and submitting fixes to gcc to make this (partially, maybe) work. Since nobody seems to be actively using libstdc++ on AVR now, it is likely to be broken in upstream gcc. It would be interesting if Arduino enabled it, so there will be an active userbase to keep the code working maybe? But, this is really an entirely different discussion, that might be more suited to an issue in the toolchain-avr repository...

I'm also wondering under which circumstances does std::function call new / malloc() on supported platforms?

It can store a function pointer directly, but if the function should capture any arguments (i.e. with std::bind, a non-static method or a lambda that has captures), then it uses dynamic memory for the arguments (IIRC there is no storage included for any arguments).

Dynamic memory is certainly a downside of std::function. In another project, I implemented a simpler version of std::function that has a fixed memory amount for arguments and never uses dynamic memory (which means more memory usage with fewer arguments, but at least makes memory predictable), but the downside is that (AFAICS) you can never make something like this convertible from or to std::function (except by adding wrappers, but that just increases memory usage even more), so using it is tricky wrt compatibility.

@PaulStoffregen
Copy link

Is that simpler, static-only std::function code open source? Would love to take a look, maybe even use it for Teensy if that's ok?

@vChavezB
Copy link
Author

vChavezB commented Mar 8, 2022

What about an alternative for std::function such as discussed by Mikael Rosbacke in his presentation "Embedded friendly std::function alternative".

Github repo

@matthijskooijman
Copy link
Collaborator

Is that simpler, static-only std::function code open source? Would love to take a look, maybe even use it for Teensy if that's ok?

Nope, I wrote it for a customer, but I think I can convince them to release this as open. I just keep forgetting to bring it up when I'm there, but I'll get back to you.

What about an alternative for std::function such as discussed by Mikael Rosbacke in his presentation "Embedded friendly std::function alternative".

That's an interesting approach, but that does not seem to include storage for e.g. lambdas with captures or bigger callable objects/functors. It does seem to be able to store method pointers (i.e. store the this pointer in addition to the method pointer), and does provide a unified API for "callable things", so it might still be interesting (even if it does not solve the storage of lambdas with captures, if you manage that storage yourself, you can use them).

Thinking on this a bit more, I think that this "delegate" class is essentially the same as the commonly used "function pointer" + `void* arg combination, except that it neatly wraps both in a single value and handles the packing and unpacking so you can just pass a (stored) lambda and/or method pointer and not deal with the details.

@PaulStoffregen
Copy link

Must admit, hadn't heard of delegate before. Looks very interesting. But I'd really like to ability to pass just one 32 bit (or pointer size) input to the callback. Can't have that with delegate, right?

@vChavezB
Copy link
Author

vChavezB commented Apr 5, 2022

Just to continue the discussion, referring to the comment from @PaulStoffregen on another pull request in the avr core.

This example code calls Serial.print() from within the callback, which unintentionally illustrates the main reason why callbacks directly from interrupts are not a good design for the Arduino API.

That is true. If the API user calls blocking functions or does not follow best practices for ISRs then you could run into all sorts of problems. This raises for me the question, are users responsible for calling blocking functions, non-reentrant functions, etc? Because if this is the case you can just mention this in the documentation and it's up to the user to follow best practices. If not you could implement design by contract (e.g., Asserts) to verify that the user does not call Arduino API functions inside the user-defined ISR callback.

I'm not against callback. In fact, I want to see this API. But it really should be done in another way where the interrupt merely sets a flag or other mechanism for the callback to happen later, from the main program context (eg, made from yield or an RTOS mechanism).

Yes, I think that in general, an ISR should merely notify of events. However, not all Arduino cores use an RTOS. I think the relevance of giving the user the option to register an ISR callback, is that you could have direct access to hardware events. This is not useful for basic users who do not have a clue what an ISR is or don't have experience with embedded systems.

Now you can say, well if you are an embedded developer why are you using the Arduino framework? The Arduino SDK allows me to develop prototypes without the need to design and implement my own HAL each time I use a different evaluation board. For this reason, an ISR callback can be considered a feature that should be used only by advanced users.

@vChavezB
Copy link
Author

vChavezB commented Apr 5, 2022

it neatly wraps both in a single value and handles the packing and unpacking so you can just pass a (stored) lambda and/or method pointer and not deal with the details.

Yes, exactly it wraps nicely callbacks for C++.

but that does not seem to include storage for e.g. lambdas with captures or bigger callable objects/functors.

@matthijskooijman Could you elaborate further on what this use case would look like?

I made an example based on the presentation, with compile explorer, to show how it looks. Perhaps you can modify it to show how you expect to register functors or lambdas?

The author also includes test cases for the delegate repository. It includes how lambdas and functors [1],[2], can be registered for callbacks.

@vChavezB
Copy link
Author

vChavezB commented Apr 9, 2022

Another alternative is the Embedded Template Library, which is similar to the delegate repository I talked about.

https://www.etlcpp.com/delegate.html

It has the delegate template as well and is already ported to Arduino
https://github.com/ETLCPP/etl-arduino

@vChavezB
Copy link
Author

As this issue has not been followed I will close it and leave it for historical reasons so people can know some alternatives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants