-
Notifications
You must be signed in to change notification settings - Fork 12
Enhance GPIO to allow use in non-templated classes and variables #14
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
Open
lintondf
wants to merge
4
commits into
mikaelpatel:master
Choose a base branch
from
lintondf:IGPIO
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Please add benchmark when passing as parameter. This will give some insight to the current design pattern. |
Please explain changes ”const party”. What is the improvement? The class does not have state. |
My newly added IGPIO.h adds the pure virtual interface class IGPIO and the
VGPIO class which subclasses the interface and your standard GPIO class.
I have committed additional changes. Please look at BenchmarkV.ino which
replicates your Benchmark.ino but using the VGPIO class. Benchmark.ino
does the same for IGPIO and illustrates using an IGPIO const reference to
access a VGPIO object.
…On Sat, Jan 25, 2020 at 7:09 PM Mikael Patel ***@***.***> wrote:
Please add benchmark when passing as parameter. This will give some
insight to the current design pattern.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#14?email_source=notifications&email_token=ACBVP7MUUZNIAF3NUMA47JTQ7TID5A5CNFSM4KLE2S42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ5IREA#issuecomment-578455696>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACBVP7IARPWFOJMHGZIK44LQ7TID5ANCNFSM4KLE2S4Q>
.
|
Since your hardware-specific GPIO classes have no state, every member
function was already 'const' in practice, adding the declaration simply
recognizes the fact.
I ran your benchmark before and after and observed no impact. However, by
adding these declarations we can pass const references to derived objects
which the compiler can better optimize.
…On Sat, Jan 25, 2020 at 7:24 PM Mikael Patel ***@***.***> wrote:
Please explain changes ”const party”. What is the improvement? The class
does not have state.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#14?email_source=notifications&email_token=ACBVP7IBI2DJSRDAIPVKJKTQ7TJ4DA5CNFSM4KLE2S42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ5IXNQ#issuecomment-578456502>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACBVP7OPBALE7V7MOQKHYNTQ7TJ4DANCNFSM4KLE2S4Q>
.
|
There is a need for a benchmark that uses the VPIO by-reference. This will force the compiler to generate the virtual table and the virtual function call over-head will be more obvious. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Defines a pure virtual base class IGPIO from which the hardware-specific GPIO classes are derived. This allows references to IGPIO objects to be passed to non-templated class constructors and methods and allows class and other variables to be IGPIO methods without the use of templates.
Since none of the methods of the GPIO classes modify any class attributes I added const declarations throughout.
Neither the size or speed of the native hardware-specific GPIO classes are affected.
I added new benchmark methods to demonstrate the performance of the wrapped objects.