arduino / arduino/ArduinoCore-API

attachInterrupt implementation remarks

Đang mở
#99 1 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
C++
Star
306
Fork
150
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

In this repo, some additions to `attachInterrupt` have been made to allow passing an extra argument. See https://github.com/arduino/ArduinoCore-API/blob/master/api/Interrupts.h#L19-L34

I have a few remarks/questions about his:
- I do not really like the use of a container in dynamic memory here and wonder if that is really needed. On second reading, I see that passing a *pointer* to `attachInterrupt` already skips the wrapper lambda and dynamic memory, it is just other types that need it. Technically, the wrapper lambda and container only serve to convert a pointer to a reference argument, which I suspect is actually a no-op on most architectures (but that's probably hard to guarantee...).
- The implementation currently seems to not free the container on detachInterrupt, which leads to memory leaks. I can't quickly think of an easy way to fix this either, without modifying the underlying interrupt handler table to somehow mark these interrupts as dynamic.
- The argument is passed by reference and then a pointer to it is taken and stored in the container. I'm wondering if this API is understood well enough, or if people would end up passing local variables resulting in all kinds of breakage. Possibly people expect to be able to pass the argument by value, so perhaps that would be a better way to implement this? As an optimization, values that are exactly as big as a pointer could be passed without the wrapper and container, but that must assume things about the architecture-specific calling conventions, so might not be acceptable here.
- ~~AFAICS the API repo provides `attachInterrupt()` and the underlying core provides `attachInterruptParam`. However, the `attachInterrupt()` method now accepts a parameter, so the naming distinction is weird. I guess the distinction is really between the sketch-visible `attachInterrupt()` API and the internal API-to-core `attachInterruptParam()` API. So perhaps the latter should be renamed to `attachInterruptInternal()` or something similar? There might be other APIs where something like this also happens that could benefit from a clear naming convention as well.~~ Update: Seems that only a C++ overload for `attachInterrupt` *with* param is implemented, so a core needs to implement both with and without param versions. Also, `attachInterruptParam()` should probably be user-visible, to allow using it from within C code.
- AFAICS there is no `attachInterrupt()` without the extra parameter defined here, so I assume the core is expected to define that directly? For consistency, it might be better to have *all* sketch-visible versions defined here, and define only "Internal" versions in the core (where `attachInterrupt()` without the extra param defined here will then simply call the equivalent internal version without adding anything). Alternatively, the core could define *only* a param version which is then used without the non-param version, but that either requires an extra wrapper lambda (passing the original function in the extra param) resulting in extra runtime delay, or passing a param to a function that does not accept it (which is probably acceptable on register-based architectures, but cannot be guaranteed everywhere. There has been some discussion about this in https://github.com/arduino/ArduinoCore-avr/pull/58 and https://github.com/arduino/Arduino/pull/4519). So perhaps the current approach (letting the core define both a param and param-less version) is actually the best, since then the core can decide to handle both in the same way if acceptable on that architecture, or store the validity of the argument separately if needed).

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Hướng nghiên cứu

Bắt đầu với các dòng 19-34 của api/Interrupts.h và so sánh các overload attachInterrupt hiển thị với sketch với phần triển khai attachInterruptParam hướng tới core. Xem lại các cuộc thảo luận ArduinoCore-avr và Arduino được liên kết để xác định các ràng buộc hiện có; để được coi là hoàn tất, cần quyết định cách tiếp cận về ownership, thời gian tồn tại của đối số và cách đặt tên trong hành vi của attach và detach.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
cpp
Lĩnh vực
embedded-iot
Loại issue
Tái cấu trúc
Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức độ hoạt động
Đình trệ
Độ rõ ràng
Cần làm rõ
Mức phù hợp với người mới
20/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.