Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

adding WifiManager::isConfigPortalActive() abd WifiManager::isWebPortalActive() #1199

Merged
merged 1 commit into from
Jan 28, 2021
Merged

Conversation

cpainchaud
Copy link
Contributor

because sometimes you need to know :p !!!!!

@tablatronix
Copy link
Collaborator

tablatronix commented Jan 28, 2021

Makes sense I assumed users would do this in their own code.

@tablatronix tablatronix added the enhancement Feature Request label Jan 28, 2021
@tablatronix tablatronix merged commit 9eac697 into tzapu:master Jan 28, 2021
tablatronix pushed a commit that referenced this pull request Jan 28, 2021
@tablatronix
Copy link
Collaborator

tablatronix commented Jan 28, 2021

changed method names

    // check if config portal is active (true)
    bool          getConfigPortalActive();
    
    // check if web portal is active (true)
    bool          getWebPortalActive();

I just moved them around for documentation and to be not inline, thanks!

@tablatronix tablatronix mentioned this pull request Jan 28, 2021
1 task
@cpainchaud
Copy link
Contributor Author

@tablatronix about the inline thing : I did it for performance and also final image size. It would be such a waste to do a CALL for this!
What do you think ?

@tablatronix
Copy link
Collaborator

Does compiler not optimize this out anyway?

@tablatronix
Copy link
Collaborator

tablatronix commented Jan 28, 2021

my (bin) code is 16 bytes smaller, I have not checked the asm.

@cpainchaud
Copy link
Contributor Author

It's not always I see in the field so I usually optimize what I can by myself :D

@cpainchaud
Copy link
Contributor Author

@tablatronix Here is the generated ASM output without inline:

.LVL26:
.loc 1 166 0
l32r a2, .LC21 #, tmp48
mov.n a10, a2 #, tmp48
call8 _ZN11WiFiManager20isConfigPortalActiveEv #

Which does not exist when I am using INLINE

@tablatronix
Copy link
Collaborator

tablatronix commented Jan 28, 2021

Thanks ill look at it, I would love to move on to optimizing everything now for size and speed now that its fairly stable.

my dumbass was mistakingly testing without using the actual funcs, so they were compiled out lol geez

@Testato
Copy link

Testato commented Jan 28, 2021

@tablatronix about the inline thing : I did it for performance and also final image size. It would be such a waste to do a CALL for this!
What do you think ?

Performance or image size, it's not possible have the two togheter.
Inline duplicate the function code every time you use it, so the image size is increased.
Normally on embedded system the image size is preferred so inline is not used.
This two functions, imho, are not a critical speed function

@cpainchaud
Copy link
Contributor Author

cpainchaud commented Jan 28, 2021

Hi @Testato

My tests are showing image is small with inline, which makes sense because 1/ you have some bytes for the function itself and second some instructions to prepare each call

@cpainchaud
Copy link
Contributor Author

cpainchaud commented Jan 28, 2021

@Testato

Without inline image size is 971584
With inline it is 971576

and that is with just 1 call, if I do more calls I get 2bytes saved per call.

@tablatronix tablatronix added the Discussion Further Discussion ongoing label Jan 28, 2021
@tablatronix
Copy link
Collaborator

Thanks, all, if you want feel free to hit up the gitter to discuss in depth

@tablatronix tablatronix added this to the dev milestone Jun 21, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Discussion Further Discussion ongoing enhancement Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants