Azure / azure-iot-arduino

Azure IoT library for the Arduino
Other
168 stars 95 forks source link

Replace sdk/agentime.c with NTPClient back version #18

Closed sandeepmistry closed 7 years ago

sandeepmistry commented 8 years ago

Implementation of #17.

The simplesample_http example runs great on WiFi101 and my Adafruit Huzzah. This gives us more portability with the library for boards in the future.

@obsoleted please view and try out.

obsoleted commented 8 years ago

I'd like to hold off on this for now.

I want to avoid replacing parts of the upstream sdk when possible. For esp this results in NTP/time being configured within the library but not really usable outside the library. So if another library was calling If the NTPClient library used the internal esp implementation here that might help things but i'm not sure it is worth it for that library.

If we do go this route we probably would also want to expose the ntp configuration options to the caller (e.g. ntp servers).

But overall i'd like to investigate/think about this more before changing the library api surface and the contract between library and upstream sdk.

sandeepmistry commented 8 years ago

Ok, fair enough let's hold off.

Originally I had thought, the agenttime layer could be customized by the platform as there was a separate file to handle mbed.

We've started to thinking about a TimeProvider interface, which NTPClient could implement. Longer term, this would be a cleaner fit, as the library could accept an sslClient and timeProvider.

tameraw commented 7 years ago

Closing as it's been agreed to hold off. A backlog item has been created on our side to look into adding NTP into the platform_init.