Fix examples (Makefiles, logging, comments, readability). #46
Conversation
| cd iot-device-sdk-embedded-c | ||
| ``` | ||
|
|
||
| 1. Generate a [public/private key pair](https://cloud.google.com/iot/docs/how-tos/credentials/keys), and store it in the example's directory. |
There was a problem hiding this comment.
https://guides.github.com/features/mastering-markdown/
Look for Ordered lists. The benefit of starting all items with 1. is that Markdown will increment the number for you, so when you add/modify/delete items, you don't have to renumber.
| Follow the steps below to connect the FreeRTOS application to the MQTT bridge. | ||
|
|
||
| Before you begin, generate a [public/private key pair](https://cloud.google.com/iot/docs/how-tos/credentials/keys), store the private key in the `examples/freertos_linux/Linux_gcc_gcp_iot` directory, and name the key `ec_private.pem`. | ||
| 1. Go to the repository's root directory. |
There was a problem hiding this comment.
This is an instruction. Reading it I thought I have to figure out something, although this just describes the effects of the command below. I'd just keep the style: run this please, btw: this is the result. @soyrice PTAL
| ``` | ||
| cd examples/freertos_linux/Linux_gcc_gcp_iot \ | ||
| make | ||
| make clean_all |
There was a problem hiding this comment.
clean_all, too drastic, although it is necessary. Since we don't know what was the previous mbedtls build: 32bit or 64bit. Still I'd give more automagical experience than forcing clean_all always.
There was a problem hiding this comment.
I agree, that it is too drastic, however this is the Makefile for the examples.
We can assume that people are building the examples when they check out the project and start evaluating. There, it's very bad if they hit build errors.
The degraded UX is when someone already has a build, then builds the example, then their code doesn't build any more, because the wrong mbedTLS library is there. However this path is less likely at this point of the project.
So I compare a +20sec buildtime against a +N mins/hours debugging.
Nevertheless, in another PR I can fix the mbedTLS library output, so the time window that this risk is there would be small.
WDYT?
There was a problem hiding this comment.
Unfortunately this means that if I build the library with a specific CONFIG first, like all of our documentation tells the user to do, and THEN build the examples, that the special config is wiped out and replaced with a library built with the default config
I would rather solve this issue another way. Create new library target path for the 32bit and one for 64-bit libs of mbedTLS, and have the example depend on these directly, building them if necessary or at least alerting the user to their absence.
…-sdk-embedded-c into readability_fixes
…t-edge-sdk-embedded-c into readability_fixes
| iotc_connect(context_handle, /*username=*/NULL, /*password=*/jwt, | ||
| /*client_id=*/iotc_device_path, connection_timeout, | ||
| /* 4. Connect to GCP IoT Core MQTT Bridge. */ | ||
| iotc_connect(context_handle, /*username=*/"unused", /*password=*/jwt, |
There was a problem hiding this comment.
This needs to be pulled in, this bug caused our project a lot of grief figuring out that the NULL parameter for the username was tanking our connection routine @atigyi
|
@verkri I love some of the work you've done here, and I think it really improves the project. Would you be open to resolving the file conflicts so we can resurrect your efforts and get this merged in? |
|
Hi @verkri thank you for pointing out these issues, do you still want to merge this PR if so could you submit a no-op change so Travis CI could run its tests, if not i'll close out the PR. |
Reason for this PR: I was playing with the examples and found many issues that potential clients might hit as well. These are:
I went on and moved all IoT Core parameters into a struct for better encapsulation, which resulted also in a refactor in the common libraries.