Skip to content

Improve FunctionPointer class #1783

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Improve FunctionPointer class #1783

wants to merge 5 commits into from

Conversation

geky
Copy link
Contributor

@geky geky commented May 25, 2016 •
edited
Loading

  1. Currently the existing APIs are difficult to work through when composing classes with callbacks. Currently function pointers must be duplicated to pass through callback APIs. This level of indirection needs to be duplicated across any classes that pass callbacks.
  2. The callback interface is currently not standardized. This is notable in differences between APIs such as Ticker and Thread. The current FunctionPointer does not support the existing callback types and can’t be standardized as is. Additionally this has resulted in FunctionPointer being duplicated across the developer site (example).
  3. FunctionPointer is not forward compatible with C++11. Even if we hold off for now, eventually compatibility with std::function will eventually become a requirement. Libraries have already started to use the FunctionPointerArgN classes. By introducing a forward and backward compatible function pointer class now, we can reduce the confusion caused by the required name change for C++11 later.

Here are the proposed changes for the FunctionPointer class:

  • Adopt C++11 style template arguments, requires rename to Callback
  • Add constructor for C style callback functions
  • Add constructor for passing Callbacks
  • Add static function for passing to C style callbacks
  • typedef FunctionPointer/FunctionPointerArg1 for backwards compatibility

cc @pan-, @sg-, @c1728p9
mv #94, #131, #139

geky added 5 commits May 25, 2016 16:14
@geky
Add Callback, an improved FunctionPointer class
a9a7fe8 
- Adopt C++11 style template arguments, requires rename to Callback
- Add constructor for C style callback functions
- Add constructor for Callbacks
- Add static function for passing to C style callbacks
@geky
Add backward compatiblity for FunctionPointer class using Callback
e33eb27 
effectively:
typedef Callback<R(A)> FunctionPointerArg1<R,A>
typedef Callback<R()> FunctionPointerArg1<R,void>
typedef Callback<R()> FunctionPointer
typedef Callback<R()> event_callback_t
@geky
Adopt Callback class in hal
2b690e3
@geky
Adopt Callback class in rtos Threads
d796b7a
@geky
Update Callback to fix fault in serial interrupts
2e42f98 
per @c1728p9

Update the Callback class to handle a NULL thunk by returning 0
rather than trying to call the thunk.  This fixes a crash that occurs
on some targets when the TX uart handler is not attached.

Background:
The K64F HAL uart implementation calls the TX interrupt handler
every time a uart interrupt occurs while the TX register is empty.
It does not check to see if the TX interrupt has been enabled.
This means that the TX interrupt can and typically does get
run on RX events.  This causes a crash with the newer callback
code which did not (prior to this patch) support a NULL thunk.
@geky
Copy link
Contributor Author

geky commented May 25, 2016

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=ALL

@mbed-bot
Copy link

[Build 404]
FAILURE: Something went wrong when building and testing.

@c1728p9
Copy link
Contributor

c1728p9 commented May 26, 2016

Looks like there might be a problem with this patch and the async api.

@c1728p9
Copy link
Contributor

c1728p9 commented May 26, 2016

Moved to #1793 since @geky is away

@c1728p9 c1728p9 closed this May 26, 2016
0xc0170 added a commit that referenced this pull request May 27, 2016
@0xc0170
Merge pull request #1793 from theotherjimmy/callback
c3d9192 
mv #1783(Improve FunctionPointer class) + minor compatibility fix
geky added a commit to geky/mbed that referenced this pull request Jul 18, 2016
@geky
Marked FunctionPointer and friends as deprecated
ad07ab8 
FunctionPointer/FunctionPointerArg0/FunctionPointerArg1 has been
replaced by the more flexible Callback template class.

For the motivation behind adopting the Callback class:
ARMmbed#1783
@geky geky mentioned this pull request Jul 18, 2016
Merged
deepakvenugopal added a commit to deepakvenugopal/mbed-os that referenced this pull request Aug 22, 2018
@deepakvenugopal
Squashed 'features/nanostack/sal-stack-nanostack/' changes from 7963594…
94f8210 
…..2535a6c

2535a6c Merge branch 'release_internal' into release_external
6bc9e00 Merge pull request ARMmbed#1783 from ARMmbed/mac_fix
8b1577e Fixed possible memory leak at mac pd_sap_ind() function.

git-subtree-dir: features/nanostack/sal-stack-nanostack
git-subtree-split: 2535a6c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
needs: work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@geky @mbed-bot @c1728p9