RAKWireless / rak_common_for_gateway

215 stars 126 forks source link

Is lora_pkt_fwd.c really able to settimeofday? #14

Closed hdtodd closed 4 years ago

hdtodd commented 4 years ago

Line 2705 in lora_pkt_fwd.c generates the following compiler warning (my line #'s differ) on Raspberry Pi Buster running a RAK2245 PiHat.

src/lora_pkt_fwd.c: In function ‘modify_os_time’:
src/lora_pkt_fwd.c:2773:15: warning: implicit declaration of function ‘settimeofday’; did you mean ‘gettimeofday’? [-Wimplicit-function-declaration]
     int ret = settimeofday(&tv, NULL);
               ^~~~~~~~~~~~
               gettimeofday

The settimeofday() procedure does seem to be defined in <sys/time.h> in the same way as gettimeofday() is, so I don't know why there's a warning. But the implication is that the program really won't set the system time of day from GPS. (I can't test as my GPS isn't working.)

RAKWireless commented 4 years ago

@hdtodd thank you so much for your suggestion.

This is a warning that also puzzles me. Use "man settimeofday" we can view settimeofday, and we can also view settimeofday in the /usr/include/arm-linux-gnueabihf/sys/time.h file.

We will continue to focus on this issue and resolve it as soon as possible.

hdtodd commented 4 years ago

Thank you. I'm also puzzled by the warning. The invocation of settimeofday() seems to match the man specification, and the definition is in the time.h file. But the warning suggests that the compiler might have generated an invocation of gettimeofday() in place of settimeofday(), for which it couldn't find a definition.

If the compiler did that, the program would not be able to actually set the time.

I'll be curious to see what you learn on further exploration.

hdtodd commented 4 years ago

I did a little more exploring on my own. I believe the call to settimeofday() in the form used in this code was invalidated in Gnu C Library 2.31. Check this link: https://lwn.net/Articles/811275/ Other references to "gnu library 2.31" say similar things.

The recommendation is to use "clock_settime" to set the system time now.

Again, I can't test because my GPS isn't working, but I think the code you want to use looks like this in modify_os_time:

struct timespec y;
struct timespec tv;          // note the change in type here

...

tv.tv_sec = y.tv_sec;
tv.tv_nsec = 0;                // note that this is now nanosec, not microsec
//    int ret = settimeofday(&tv, NULL);                                                                                        
int ret = clock_settime(CLOCK_REALTIME, &tv);  // replaces line above

At least this compiles without a warning and presumably loads the correct library routine to set the time.

It would probably be a good idea to check if ret!=0 and print the appropriate error message for whatever is preventing this from setting time if there is an error, or at least say that the time hasn't been set.

Again, I can't test it yet, but I did install it in my code and will test it when my replacement RAK2245 arrives. I'll probably add the error-handling code then, too, if you haven't.

I hope this helps.

David

RAKWireless commented 4 years ago

@hdtodd I also thought that it might be caused by the version of the lib library. I will follow your suggestions to continue testing.

RAKWireless commented 4 years ago

@hdtodd We have updated the dev branch to remove this warning. Thank you again.