Azure / azure-iot-arduino

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

Reference to AzureIoTHubClient in SimpleSample code appears not necessary #51

Closed GregTerrell closed 7 years ago

GregTerrell commented 7 years ago

I am probably missing something, but when testing with the SimpleSample MQTT code I discovered that the references to AzureIoTHubClient appear to do nothing for the solution.

static AzureIoTHubClient iotHubClient; 
...
void setup() {
    unsigned long epochTime = 0;

    initSerial();
    initWifi();
    initTime();

    iotHubClient.begin(sslClient);
}

I followed the logic into the IoTHub code and searched IoTHub files for any reference to the AzureIoTHubClient class, without finding any purpose to this class. With that experience, I commented out the iotHubClient.begin(sslClient) line; this did not break the successful operation of the SimpleSample code example or my project based from it.

Is there a reason for AzureIoTHubClient and the invocation of the .begin() method?

Thanks, Greg

mamokarz commented 7 years ago

Hi @GregTerrell ,

Thanks for your question, you are right, it is not necessary anymore. We can safely remove, not only the AzureIoTHubClient definition, but the sslClient as well, from both SimpleSample_MQTT and SimpleSample_HTTP.

// In the next line we decide each client ssl we'll use.
#ifdef ARDUINO_ARCH_ESP8266
static WiFiClientSecure sslClient; // for ESP8266
#elif ARDUINO_SAMD_FEATHER_M0
static Adafruit_WINC1500SSLClient sslClient; // for Adafruit WINC1500
#else
static WiFiSSLClient sslClient;
#endif

static AzureIoTHubClient iotHubClient;
iotHubClient.begin(sslClient);

If you would like to contribute with a pull request, I’ll be glad to accept and merge it.

Thanks, Marcos

GregTerrell commented 7 years ago

Sorry for the delay... yes I will prepare a pull-request in the next day or two for the removal. Looks like it affects the items in the examples folder for azure-iot-arduino, azure-iot-arduino-protocol-http, and azure-iot-protocol-mqtt.

tameraw commented 7 years ago

PR merged.