Advanced Users can post their questions and comments here for the not so Newbie crowd.

Moderator: eriksl

User avatar
By davydnorris
#81389
eriksl wrote:I never did the trick on prototypes in header files. I feel it doesn't belong there. Where the function is linked to is of no concern to the code that calls the function, i.e. it should not be a part of the interface/contract.


Exactly - and the compiler ignores them when they're in the header file anyway so they can be a red herring. People see the definition and think it's being applied.

eriksl wrote: And by the way, you really don't need to include c_types.h to get a function into IROM. Just add a define like this
Code: Select all#define irom __attribute__((section(".irom0_text")))
to your source file or even use the __attribute__ line directly. I hate the long, capitalised and very unclear name they assigned to their version of this macro.


I don't mind the longer definition, and it's so widely used out there I'm happy to keep using it, but it would be better to have done what they did in the RTOS SDK and set up the linker files to put everything in IROM by default. Then you only need modifiers on the IRAM functions, which are much rarer, and in most cases only in drivers.

eriksl wrote:In fact, including c_types.h is never required. It defines integer types it shouldn't define (i.e. uint8 while they should have used the types in <stdint.h>: uint8_t). I go to great lengths to remove Espressif made up integer types from all sources and include files and replace them with the proper stdint.h ones. For LWIP that wasn't such a big deal in the end, because most Espressif's code is gone now (espconn).


I have done the same, and have created a pull request on the Espressif NonOS project containing the changes to all the SDK headers. I've also updated the driver lib to use standard types but haven't pushed that yet.
User avatar
By eriksl
#81394 Nice :-)

I'm curious whether they'll respond, although I still see some activity on the NONOS SDK.
User avatar
By eriksl
#81487 As posted, I now got rid of all the Espressif SDK header files. It also allowed me to get the LWIP source files into a state that more closely resemble the original source files (before Espressif had their way with it, what a mess!)

My next project will be replacing the SNTP stuff, apparently it's not part of LWIP but added by Espressif, as to be seen from the quality of the code, also such a mess.

They've done it much too complex, probably they copied and pasted some existing implementation and changed it until it more or less worked.

I prefer to make my real own implementation from scratch, that only does what it needs to do and nothing more, and do it really well. E.g. keep synchronising and not only do it once. Also I don't think an IoT device has a requirement to sync to various servers, pick some of them to be "active" etc. It only needs to query the server once in say an hour's time, wait for the reply and simply apply it, that's all. I am also planning to implement support for multicast sntp distribution.

Please note there is quite a bit of code in the sntp.c file that really doesn't belong there, the whole unix epoch time to ascii is no SNTP business. That one I also think I can do better myself ;-)
User avatar
By davydnorris
#81493 While you're at it, it would be awesome to make the default time UTC instead of GMT+8! Never understood why they did that apart from it being China's timezone.

It would also be really good to try to add some precision adjustments into the time to get better than +-1s accuracy in the clock. It's not that hard to get down to 0.01sec resolution or even better just by by oversampling - adding some network latency calculations would make it even better, although I had a look a little while back and I don't think the hardware has some of the really useful compensation built into it, which is a shame (or at least, it's not exposed if it is).