PnP Component Sample convert sample style #1092
Conversation
momuno
added
Client
momuno
requested review from
CIPop,
ericwolz,
ewertons,
hihigupt and
jspaith
as code owners
…rm), clang-format
|
Will make a separate PR to apply all of today's updates for all samples: const, ref_, out_ |
| #pragma warning(disable : 4201) | ||
| #endif | ||
| #include <paho-mqtt/MQTTClient.h> | ||
| #ifdef _MSC_VER | ||
| #pragma warning(pop) | ||
| #pragma warning(default : 4201) |
There was a problem hiding this comment.
I think @ahsonkhan was the one that suggested using push/pop for the pragma's?
There was a problem hiding this comment.
What's the rationale/benefit of not using push/pop?
Push/pop let's you keep the warning state unmodified outside of this usage, which is nice. If you don't do that, we change the warning state to something other than what the outer-scope might have set it as (which may not be default).
There was a problem hiding this comment.
ah, ok. Am I understanding correctly that we with the push/pop, we shouldn't need to set the warning back to default, corrrect? I will update this in all the samples.
There was a problem hiding this comment.
#ifdef _MSC_VER
#pragma warning(push)
// warning C4201: nonstandard extension used: nameless struct/union
#pragma warning(disable : 4201)
#endif
#include <paho-mqtt/MQTTClient.h>
#ifdef _MSC_VER
#pragma warning(pop)
#endif
There was a problem hiding this comment.
ah, ok. Am I understanding correctly that we with the push/pop, we shouldn't need to set the warning back to default, corrrect?
yes
The pop should happen near the end of the file or after the code block that is causing the warning you want to disable.
| connect_mqtt_client_to_iot_hub(); | ||
| LOG_SUCCESS("Client connected to IoT Hub."); | ||
|
|
||
| subscribe_mqtt_client_to_iot_hub_topics(); |
There was a problem hiding this comment.
nit: should this be
| subscribe_mqtt_client_to_iot_hub_topics(); | |
| subscribe_mqtt_client_to_iot_pnp_topics(); |
curious on your opinion
There was a problem hiding this comment.
hmmm. i'm not sure. ultimately, it is connecting to the hub. @ericwol-msft and @jspaith what do you think?
There was a problem hiding this comment.
not a big deal feel free to ignore for this PR
Updates to includes, defines, logging, const, etc.
Refactor of MQTT message initialization into component file.
Still needs to update readme documentation and pnp component sample c file to match.