Skip to content

Remove dependency with the core #2

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

Merged
merged 1 commit into from Sep 14, 2017
Merged

Remove dependency with the core #2

merged 1 commit into from Sep 14, 2017

Conversation

ghost
Copy link

@ghost ghost commented Sep 6, 2017

I added an update function to the Ethernet class to remove the dependency inside the main.cpp.
The library is now completely independent of the core.
The function update() updates the LwIP stack also.

* This function updates the LwIP stack and can be called to be sure to update
* the stack (e.g. in case of a long loop).
*/
void EthernetClass::update(void)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need this new update interface ? as far as I understood the list of callbacks is a table in core ... so stm32_eth_scheduler will be called directly .... isn't it ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right @LMESTM .
I think @fprwi6labs add it to enhance EthernetClass API that user can use in his sketch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how to use update is not clear to me - please add more information in the commit message why we make the change and how this will/can be used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is just in case you go in a blocking loop inside the loop() function, neither updateCoreCallback nor stm32_eth_scheduler will be called. So I just give a possibility to the user to update the LwIP stack to keep Ethernet alive.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - I'd like to see this kind of information in the commit message as well.
Typically I guess that you don't want both APIs to be called, user should select one of them ? What is the timing of a loop that requires this API to be called from user application ?
Other point : is update a standard name, or could be consider EthernetClass::schedule() ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User can use both method.
I don't master enough the LwIP stack to give you a figure.
There isn't a standard name so I can change.

@@ -45,6 +45,7 @@
#include "lwip/dhcp.h"
#include "lwip/prot/dhcp.h"
#include "lwip/dns.h"
#include "core_callback.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid any build issue if user do not use the next core version and ensure compatibility, it should be fine to do something like that? Tested and it's ok.

#if __has_include("core_callback.h")
#include "core_callback.h"
#else
void registerCoreCallback(void (*func)(void)) {
  UNUSED(func);
}
#endif

@ghost ghost requested a review from fpistm September 14, 2017 08:12
@fpistm fpistm merged commit 93d4d80 into stm32duino:master Sep 14, 2017
@nopnop2002 nopnop2002 mentioned this pull request Jul 24, 2019
ABOSTM pushed a commit to ABOSTM/STM32Ethernet that referenced this pull request Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants