Page 1 of 1
Any way to override ESP IDF constant #defines
Posted: Mon Feb 26, 2018 6:54 am
by markwj
Is there any way to override ESP IDF constant #defines, to be specific to the app we are developing (built against ESP IDF).
For example, components/lwip/include/lwip/port/lwipopts.h I need a different implementation for #define SNTP_SET_SYSTEM_TIME_US.
Any suggestions appreciated.
Re: Any way to override ESP IDF constant #defines
Posted: Mon Feb 26, 2018 2:49 pm
by kolban
If we look at the code we see that it "is what it is". That means that the only way to be sure that the all the code sees your replaced implementation is to actually change the source of lwipopts.h to be your new definition.
My opinion is that it is "ok" to edit/change this source and build your own app. The down-side is that when you replace the ESP-IDF, you will need to manually fix-up your modified source. This means keeping a good record of what you changed. If your own source for your project is open source, you'll need to document the changes that you expect to be performed by users of your own project.
Re: Any way to override ESP IDF constant #defines
Posted: Tue Feb 27, 2018 1:35 am
by ESP_Sprite
Alternatively, copy-paste the component you've changed into your own project and maintain it there; that means you can check it into whatever versioning system your project uses; it also means that your users don't have to faff around changing their ESP-IDF tree.
If you think being able to configure this define is interesting for other users as well, you can also create an issue or pull request on Github, and we'll see if we can modify the main esp-idf tree.
Re: Any way to override ESP IDF constant #defines
Posted: Tue Feb 27, 2018 1:43 am
by markwj
Thanks for the replies. I was really just wondering if there is a standard way of overriding these things.
Given that we already have our own fork of ESP IDF, it is not too onerous to change it, but we don't want to diverge from mainline too much. We'll make the change in an extensible way, then send a pull request to see if it can be merged back into Espressif mainline.
Seeing this thread:
viewtopic.php?f=2&t=4695
There are at least two other people that want this specific functionality.
Re: Any way to override ESP IDF constant #defines
Posted: Tue Feb 27, 2018 2:25 am
by ESP_Angus
markwj wrote:
Given that we already have our own fork of ESP IDF, it is not too onerous to change it, but we don't want to diverge from mainline too much. We'll make the change in an extensible way, then send a pull request to see if it can be merged back into Espressif mainline.
That would be fantastic, your PR will probably be accepted!
Some kind of weak-linked "after SNTP update" function, which is a no-op in the LWIP component but can be implemented to do something in the project, could work well. Let me know if you'd like me to elaborate on this suggestion.
Re: Any way to override ESP IDF constant #defines
Posted: Tue Feb 27, 2018 6:49 am
by OneTwo
markwj wrote:Is there any way to override ESP IDF constant #defines, to be specific to the app we are developing (built against ESP IDF).
For example, components/lwip/include/lwip/port/lwipopts.h I need a different implementation for #define SNTP_SET_SYSTEM_TIME_US.
Any suggestions appreciated.
Have you tried with
Code: Select all
#ifdef X
#undef X
// OR
#define X = Y
#define DEF_OVERWRITTEN
#else
...
#endif
Re: Any way to override ESP IDF constant #defines
Posted: Tue Feb 27, 2018 7:25 am
by markwj
ESP_Angus wrote:That would be fantastic, your PR will probably be accepted!
Some kind of weak-linked "after SNTP update" function, which is a no-op in the LWIP component but can be implemented to do something in the project, could work well. Let me know if you'd like me to elaborate on this suggestion.
My use case is to be informed when the NTP time is updated. It would also be nice to be able to see that update and control it in some way.
I think what you are suggesting is this:
Code: Select all
diff --git a/components/lwip/apps/sntp/sntp.c b/components/lwip/apps/sntp/sntp.c
index 6bee032f..e3dee911 100644
--- a/components/lwip/apps/sntp/sntp.c
+++ b/components/lwip/apps/sntp/sntp.c
@@ -198,6 +198,10 @@ static ip_addr_t sntp_last_server_address;
static u32_t sntp_last_timestamp_sent[2];
#endif /* SNTP_CHECK_RESPONSE >= 2 */
+#pragma weak sntp_setsystemtime
+
+extern void sntp_setsystemtime(u32_t, u32_t);
+
/**
* SNTP processing of received timestamp
*/
@@ -217,7 +221,7 @@ sntp_process(u32_t *receive_timestamp)
SNTP_SET_SYSTEM_TIME_US(t, us);
/* display local time from GMT time */
LWIP_DEBUGF(SNTP_DEBUG_TRACE, ("sntp_process: %s, %"U32_F" us", ctime(&tim), us));
-
+ if (sntp_setsystemtime != NULL) sntp_setsystemtime(t, us);
#else /* SNTP_CALC_TIME_US */
/* change system time and/or the update the RTC clock */
That meets my requirement. Was it the sort of thing you were suggesting?
Re: Any way to override ESP IDF constant #defines
Posted: Tue Feb 27, 2018 7:26 am
by markwj
OneTwo wrote:Have you tried with
Code: Select all
#ifdef X
#undef X
// OR
#define X = Y
#define DEF_OVERWRITTEN
#else
...
#endif
Yes. That works for our code, but doesn't seem to work for the components in the ESP IDF code tree. The #define is used inside the lwip sntp component, not in our code.
Re: Any way to override ESP IDF constant #defines
Posted: Tue Feb 27, 2018 10:57 pm
by ESP_Angus
markwj wrote:ESP_Angus wrote:That would be fantastic, your PR will probably be accepted!
Some kind of weak-linked "after SNTP update" function, which is a no-op in the LWIP component but can be implemented to do something in the project, could work well. Let me know if you'd like me to elaborate on this suggestion.
My use case is to be informed when the NTP time is updated. It would also be nice to be able to see that update and control it in some way.
I think what you are suggesting is this:
Code: Select all
diff --git a/components/lwip/apps/sntp/sntp.c b/components/lwip/apps/sntp/sntp.c
index 6bee032f..e3dee911 100644
--- a/components/lwip/apps/sntp/sntp.c
+++ b/components/lwip/apps/sntp/sntp.c
@@ -198,6 +198,10 @@ static ip_addr_t sntp_last_server_address;
static u32_t sntp_last_timestamp_sent[2];
#endif /* SNTP_CHECK_RESPONSE >= 2 */
+#pragma weak sntp_setsystemtime
+
+extern void sntp_setsystemtime(u32_t, u32_t);
+
/**
* SNTP processing of received timestamp
*/
@@ -217,7 +221,7 @@ sntp_process(u32_t *receive_timestamp)
SNTP_SET_SYSTEM_TIME_US(t, us);
/* display local time from GMT time */
LWIP_DEBUGF(SNTP_DEBUG_TRACE, ("sntp_process: %s, %"U32_F" us", ctime(&tim), us));
-
+ if (sntp_setsystemtime != NULL) sntp_setsystemtime(t, us);
#else /* SNTP_CALC_TIME_US */
/* change system time and/or the update the RTC clock */
That meets my requirement. Was it the sort of thing you were suggesting?
Something like this.
You can use weak linking directly so you don't need the pointer check, something like this (pseudocode-ish):
lwipopts.h:
Code: Select all
/* Weak-linked default calls settimeofday(). Implement a custom sntp_update_time()
in your own code to change this behaviour.
*/
void sntp_set_system_time(struct timeval *tv);
#define SNTP_SET_SYSTEM_TIME_US(sec, us) \
do { \
struct timeval tv = { .tv_sec = sec, .tv_usec = us }; \
sntp_update_time(&tv, NULL); \
} while (0);
In lwip/apps/sntp/sntp.c
Code: Select all
/* Default implementation calls settimeofday() directly */
void __attribute__((weak)) sntp_set_system_time(struct timeval *tv)
{
settimeofday(tv);
}
Then, in your code somewhere
Code: Select all
void sntp_set_system_time(struct timeval *tv)
{
printf("Look ma, I hooked the function!\n");
settimeofday(tv);
}
The only gotcha with weak-linking in ESP-IDF is that you have to give the linker a reason to examine the source file where you have your "overridden" version of sntp_set_system_time(). This means the same source file has to contain another function which is called by your app, or another symbol (a global variable or something) which is referenced elsewhere in the code. Otherwise the linker won't ever look in that file, and you'll get the default sntp_set_system_time().
Re: Any way to override ESP IDF constant #defines
Posted: Thu Mar 01, 2018 1:06 am
by markwj
Thanks for the advise, and pointers.
I have implemented something along the lines you propose, and it works well for my use-case. I ended up simply moving the SNTP_SET_SYSTEM_TIME_US(t, us) into a sntp_setsystemtime_us(u32_t t, u32_t us) weak linked function (as well as doing the same with SNTP_SET_SYSTEM_TIME and sntp_setsystemtime, for completeness). That works well, and is easily overridable in user application code.
Code: Select all
extern "C"
{
void sntp_setsystemtime_us(uint32_t s, uint32_t us)
{
... settimeofday ...
}
}