Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sendPendingGetTwinRequests only after the suback is received #1585

Merged
merged 3 commits into from
Jul 9, 2020

Conversation

ericwolz
Copy link
Contributor

@ericwolz ericwolz commented Jul 9, 2020

Checklist

  • I have read the [contribution guidelines] (https://github.com/Azure/azure-iot-sdk-c/blob/master/.github/CONTRIBUTING.md).
  • I added or modified the existing tests to cover the change (we do not allow our test coverage to go down).
  • If this is a modification that impacts the behavior of a public API
    • I edited the corresponding document in the devdoc folder and added or modified requirements.
  • I submitted this PR against the correct branch:
    • This pull-request is submitted against the master branch.
    • I have merged the latest master branch prior to submission and re-merged as needed after I took any feedback.
    • I have squashed my changes into one with a clear description of the change.

Reference/Link to the issue solved with this PR (if any)

Description of the problem

Description of the solution

@ericwolz ericwolz merged commit db4875c into master Jul 9, 2020
@ericwolz ericwolz deleted the Twinrequest_timing_issue2 branch July 9, 2020 23:11
ericwolz added a commit that referenced this pull request Jul 31, 2020
* enable e2e testing

* linux_cares.sh change

* release_2019_10_07_after_bump_version

* Fix segfault in the function message_queue_move_all_back_to_pending at message_sender.c and add UT for it.

* Move longhaul timeoutInSeconds to correct place in yml, change to a snug 800-min limit

* Mqtt twin timeout cleanup (#1234)

* Check correct timeout time

* Fix MQTT timeout issues

Fix regression introduced on overly aggressive MQTT timeouts.

To fix and cleanup code a bit:
* Remove concept of enqueue time and instead have a message creation time.
* Set this message creation time ALWAYS, regardless of whether it's a PUT or GET.
* Check this publish time on timeouts.
* Create a common function to create TWIN requests and cleanup some dup'd code.

On test side:
* Fix up UT's required of course
* Change the default timer that UT uses to start at 30 minutes, not 0 minutes.
  If we had done this it would've caught initial bug where a message creation time was never init'd but close enough to 0 to have tests pass.

* Test if the provided key is valid when create an iothub client to fail fast and generate a better log. [issue #1220]

* Fix the cancellation of the longhaul jobs in the gate for Windows platforms

The longhaul tests on Windows have been cancelled by devops after 12h, because not all Windows LH tests run in parallel.
There are 10 tests, but on our scripts we restrict the max number of parallel tests to 8.
We could fix this in different ways, but this is the cheapest and most straightforward.
Also, on Linux we run up to 16 tests in parallel, so increasing by that much on Windows should be acceptable.

* Handle unexpected states in dev provisioning messagesender and messagereceiver state cb (issue #1254)

* added a define that can be used as an option to reduce memory footpri… (#1274)

* added a define that can be used as an option to reduce memory footprint on embedded devices Note: esp8266 may run out of memory without this define.

Add in #defines for loading only specific certs.

* Update linux_c_option_test.sh
Added option to test build with -Duse_baltimore_cert=ON

* Update certs.c
Update to certificate descriptions.

Name conventions set.

* Update CMakeLists.txt
pass cert defines down to compiler

* Update readme to reflect end of active dev on service SDK (#1284)

* fix setting hsm_* variable for cmake #1251

* Adding SECURITY.MD file into our repositories. (#1292)

* Remove incorrect const qualifier in serializer methodreturn

* Updating references from Device Explorer to Azure IoT Explorer. (#1299)

Updating references from Device Explorer to Azure IoT Explorer and some minor documentation changes.

* Improve *SendEventAsync documentation about how the message handle is treated (gh#1309)

Explicitly explain how the memory used by IOTHUB_MESSAGE_HANDLE is copied by the *_SendEventAsync function,
allowing the calling application to destroy the message argument right after the *_SendEventAsync function returns.

* fix linux option build sh.

* fix to include corresponding provisioning tools according to build options.

* fix #1288: correlation id in body for upload to blob (#1310)

* Update submodules

* add Visual Studio default build output to .gitignore (#1317)

* Fix GetSendStatus in AMQP transport (github issue #1039)

Relates to: #1039

The iothub client layer shares the waiting_to_send list with the AMQP transport, which then sends its elements down through individual function calls
to telemetry_messenger_send_async. In case the transport is reconnecting, it should look into waiting_to_send list instead of consulting the telemetry_messenger module.

* dawalton/wolfssl fix (#1330)

* fix bug in jenkins/raspberrypi/Dockerfile

* Removed local sample for ios, point users to WORKING Azure-Samples ios sample.

* Update CocoaPods-Samples.md

* Update CocoaPods.md

This readme does not need to specify HEADER_SEARCH_PATHS as this is already populated by the podspecs.

* Update submodules

* Adding c-utility documentation for XIO buffer configuration. (#1305)

* update submodules (#1366)

Updated modules for

azure-umqtt-c: 65cdd1013715fb9d208c42f957eb353fbe22bafb
azure-uamqp-c: 142cfab9d66c6f81ea0cceb635f31e00cfa51c77
azure-utpm-c: afe8ad192502979e2b754c7d7c0b2e7178c78d09
azure-uhttp-c: b67a6bfa0d018a8a23176ee214e46c208fc323c3
azure-c-shared-utility: 48f7a556865731f0e96c47eb5e9537361f24647c

* release_2019_12_11_after_bump_version

* Update CACertificateOverview.md

Removing the line on casing with DPS since DPS is now case-insensitive

* Fix sample dockerfile in cross-compilation documentation

* Doc explaining LL vs convenience layers (#1390)

Document difference between LL and convenience layer in a single place we can refer customers to. Fixes #1186.

* Add documentation abot how to use the SDK with the latest version of TLS only

* Address CR comments, add info in documentation to set ciphers

* Re-arrange topics in configure_tls_protocol_version_and_ciphers.md as per CR comments

* Arduino devops spike (#1395)

* Create arduino-esp.yml

* Create setup_arduino_libraries.sh

* update Arduino library dir for devops test

* adding esp_arduino_uart_interface.py for arduino_esp checkin gate

* Update arduino-esp.yml

* Create esp_input.sh

* Update arduino-esp.yml

* Update setup_arduino_libraries.sh

* Update setup_arduino_libraries.sh

* Update esp_arduino_uart_interface.py

* Fix a link to the device sdk. (#1399)

Co-authored-by: ewertons <[email protected]>

* iothub_client.h: DEPRECATED (#1122)

* tsa fixes (#1401)

* tsa fixes

Co-authored-by: bikamani <[email protected]>

* Add shebang directive to certGen.sh and make it executable (#1385)

Co-authored-by: danewalton <[email protected]>

* typo for TPM with alt registration id (#1393)

small typo for the sample to work using TPM with alternate registration id on line 266

Co-authored-by: danewalton <[email protected]>

* Add missing include for atol usage in macro (#1355)

Co-authored-by: danewalton <[email protected]>

* Update .mxchip-ci.yml for Azure Pipelines (#1387)

Co-authored-by: danewalton <[email protected]>

* Update setup_arduino_libraries.sh (#1403)

Co-authored-by: danewalton <[email protected]>

* release_2020_01_22_after_bump_version

* Update readme.md (#1410)

* Update readme.md LTS support table for release lts_02_2020

* [ACR] Update readme.md LTS support table for release lts_02_2020

* [ACR2] Update readme.md LTS support table for release lts_02_2020

* Update CocoaPods-Samples.md (#1424) for 1.3.8 LTS release

* Create CredScanSuppressions.json

add CredScanSuppressions.json to build directory for unit tests flags

* add check for NULL pointer dereference in deviceTwinCallback

* add check for NULL pointer dereference in deviceTwinCallback

* add defualt testing string

* Jbobotek patch mxchip iotz (#1431)

* Update setup.sh

* Update build.sh

* Update .mxchip-ci.yml for Azure Pipelines

* Update arduino-esp.yml for Azure Pipelines

* Update setup_arduino_libraries.sh

* Delete unnecessary .sh, update_sdk_submodule.sh

* Update setup_arduino_libraries.sh

* Update setup_arduino_libraries.sh

* Update raspi.yaml for 2 agent concurrency (#1438)

* Update raspi.yaml

* Update rpi_input.sh

* Update raspi.yaml for Azure Pipelines

* Update rpi_uart_interface.py

* Jbobotek esp new sample location (#1444)

* Update setup_arduino_libraries.sh

* Update arduino-esp.yml

* Update rpi_uart_interface.py

* Update setup_arduino_libraries.sh

* Update arduino-esp.yml for Azure Pipelines

* Update esp_arduino_uart_interface.py

* Update setup_arduino_libraries.sh (#1452)

* submodule umock update (#1460)

* update unit tests for umock aliasing

* submodule reference update

* update unit tests for umock aliasing

* update unit tests for umock aliasing

* update unit test for umock aliasing

* update unit tests for umock aliasing

* update umock register value functions for BINARY_DATA

* fix umock aliasing type

* update umock register value functions for BINARY_DATA

* update umock register value functions for data

* update submodule references

* fix unit tests

* populated BINARY_DATA shell functions

* revert to shell functions

* opt out of codesign (#1457)

* opt out of codesign

* flip to false

* add component governance

* Update .mxchip-ci.yml

* Update arduino-esp.yml

* Update linux_back_compat.yml

* Update longhaul.vsts-ci.yml

* Update mxchip-nightly-ci.yml

* Update raspi.yaml

* Update raspi.yaml

* Update .mxchip-ci.yml

* Update mxchip-nightly-ci.yml

* removing skip variable.

Co-authored-by: jbobotek <[email protected]>

* Provisioning client retry after DNS failure (#1455)

* Updating c-utility submodule to pull in DNS fix.
Fixing provisioning client state check before timing out. Fixing prov client init if no TPM server is available. Fixing sample possible race condition.
Adding prov_device_client UTs.
Fixing prov_client connected state. Adding prov_device_client_ll UTs.

* Update c-utility to 37ce4d98c7685a971ff9bdf6641c4b40d780f4ec. Adding submodule update doc.

* Malloc to Calloc calls for 0 initialize padding bytes (#1468)

* malloc to calloc, fix UTs expected calls

* malloc to calloc, fix UTs expected calls

* Update submodule references

* Cap the maximum delay between attemps of reconnection (retry policy)

This change caps the maximum delay between attempts of reconnection to thirty seconds.
Such constraint applies to the following
- Retry policies: IOTHUB_CLIENT_RETRY_LINEAR_BACKOFF, IOTHUB_CLIENT_RETRY_EXPONENTIAL_BACKOFF, IOTHUB_CLIENT_RETRY_EXPONENTIAL_BACKOFF_WITH_JITTER.
- Transports: AMQP, AMQP/WS, MQTT, MQTT/WS.

This has been a long-time request, and now it aligns with other Azure IoT SDKs (e.g., https://github.com/Azure/azure-iot-sdk-csharp/blob/f65988b6122cd43df32c123d707395d1bb4367b3/iothub/device/src/TransientFaultHandling/RetryStrategy.cs#L43)

* Change sprintf to snprintf_s

* Add windows define macro

* Add blockID range check

* Cast rand() result to double before arithmetic

* Update .vsts-ci.yml (#1471) to new agent

* Update requirements.txt to use new urllib3 (#1474)

* Add NULL dereference checks

* fixed lts naming (#1475)

* Refactor to single return point

* Fix comment typo

* Jbobotek patch python docker (#1476)

* Update requirements.txt

* Update requirements.txt (#1483)

* Update Dockerfile with new openssl (#1487)

* Test PR for raspberry pi Cross Compile step (#1462)

* Update raspi.yaml

* Update .vsts-ci.yml

* Update build.sh

* Update raspberrypi_c.sh

* Update .vsts-ci.yml for Azure Pipelines

* Update raspberrypi_c.sh

* Update .vsts-ci.yml

* Update build.sh

* Update raspberrypi_c.sh

* Update .vsts-ci.yml for Azure Pipelines

* try new buster brown

* try access shell

* Add NULL checks

* wrong include in provision sample (#1489)

* wrong include in provision sample

* Test PR for raspberry pi Cross Compile step (#1462)

* Update raspi.yaml

* Update .vsts-ci.yml

* Update build.sh

* Update raspberrypi_c.sh

* Update .vsts-ci.yml for Azure Pipelines

* Update raspberrypi_c.sh

* Update .vsts-ci.yml

* Update build.sh

* Update raspberrypi_c.sh

* Update .vsts-ci.yml for Azure Pipelines

* try new buster brown

* try access shell

* wrong include in provision sample

Co-authored-by: jbobotek <[email protected]>

* Fix for memory leak in _GetTwinAsync (gh issue #1478)

This bug is in the iothub_client_core.c layer.
Tracked by github issue #1478

* Update c-utility hash b536e0b

* Update mxchip repos for pulls (#1486)

* Update build.sh

* Update .mxchip-ci.yml

* Update build.sh

* Update .mxchip-ci.yml

* RIoT submodule update

* Update mxchip repo stuff (#1513)

* Update .mxchip-ci.yml

* Update build.sh

* refactor build files

* Fixed typo in build files

* Include all .c/.h in build file

* Add custom RIoT config file

* Update RIoT to mbedtls crypto

* Submodule update

* Update to newest RIoT

* Fix includes order

* Remove source files from build file

* Fixed path typo

* Add source file

* Add RiotCrypt includes and uint64/32 check

* Add RiotCrypt source files

* Add uint32_t root signing key

* Adjust root key initialization

* Add uint32_t check and proper initialization

* Add optional parameters in X509GetDeviceCertTBS

* Update to latest RIoT

* change order of enum for back compat (#1525)

* Add ECC initialization and RIoT cmake flag

* Update input.sh and mxchip-ci.yml to load wifi settings every time(#1515)

* Update input.sh

* Update .mxchip-ci.yml for Azure Pipelines

* Add mbedtls free APIs

* Switch to dynamic derive key

* Add signature destroy function

* Refactor to utilize RIoT new crypto lib

* Fixed extension used in leaf certs to be usr_cert extension set from openssl.cnf (#1519)

Co-authored-by: Eric Wolz <[email protected]>

* Cleanup spacing, fix UT umock functions

* Fixed UT snprintf call

* Refactor Cmake targeting order

* Fix X509GetDeviceCertTBS parameters

* Cleaned up compile definitions

* Update submodules

* add freeing function for x509 key & removed signature prototype

* Add error logging for double-free & negative test comments

* Removed config file, add cmake parameter

* Update release tag formats (#1545)

* Add UT compile definitions

* Add compile definitions for dll & lib

* Clean up TBS calls & add win32 definitions wrapper

* Add ModelID parameter to IoTHub Client (MQTT only) (#1552)

In order to support applications building PnP / DigitalTwin clients on the `master` IoTHub API, we need to let them specify their modelId for the MQTT CONNECT packet, which lives in the userName field.  

This is done via a `DeviceClient_SetOption(handle, OPTION_MODEL_ID, "dtmi:...");`.  It must be specified prior to the first CONNECT, since trying to add this or allow it to be changed later will considerably complicate the state management.  The only time a modelId will change on a given device/module identity in practice is during firmware updates in any case, which implies a new handle in any case.

The modelID parsing feature requires an API version which is still in preview and not deployed worldwide.  To account for this, we will use a preview API version if modelId has been specified and otherwise use the existing model ID.  This API version split is not long-term desirable; #1547 tracks fixing.

Note that this change is MQTT(_WS) only, per decision to only support C device SDK with PnP over MQTT(_WS).

* MSR feedback fixes

* Update API version & new test for twin array support (#1539)

IoTHub has added support for JSON arrays inside device twins.  To enable this, we need to up our API version string.  

Also extending e2e twin testing to check that arrays are handled correctly.  Even though it's all opaque to C SDK not that much extra testing.

* Update setting_up_vcpkg.md (#1559)

* Fix for memory leak if destroying device client right after sending Twin reported property update

The issue has been reported on github through #1557

* Fire callback on IoTHubClientCore_Destroy for reported state not sent

* Fix message printed in case of connection creation failure (#1569)

* chore: update support for IoT help and msft q&a options (#1564)

* chore: update support for IoT help and msft q&a options

* chore: update for msft q&a

Co-authored-by: danewalton <[email protected]>

* Fix measurement unit name in samples (#1566)

* Revert to passing TBS pointer & free function cleanup

* Update RIoT to latest

* Samples: fixed and unified comment related to setting the trusted certificate (#1530)

* Add Baked-in Dev CA Root for simulation

* Add define for mbedtls_mpi_sint

* Add new-line to EOF

* Updating string for ModelId in MQTT CONNECT (#1577)

IoTHub has changed the convention for indicating a device's ModelId.

Instead of passing `digital-twin-model-id`, we now pass `model-id`.

This succeeds on latest canary deployments.

* update logic for retry number check (#1580)

* PnP Simple thermometer sample (#1579)

Implement a simple PnP thermometer sample that implements the interface defined [here](https://github.com/Azure/opendigitaltwins-dtdl/blob/master/DTDL/v2/samples/Thermostat.json).

* Update submodules

* Adding TPMv2 Software Stack documentation (#1581)

* sendPendingGetTwinRequests only after the suback is received (#1585)

* sendPendingGetTwinRequests only after the suback is received

* Update unit tests for fix on MQTT transport/sendPendingGetTwinRequests

Co-authored-by: Ewerton Scaboro da Silva <[email protected]>

* Add TemperatureController to PnP samples (#1584)

* Add TemperatureController model for a more complex PnP sample.
* Expand the common library specifically with PnP parsing helpers.
* Fix up fit and finish issues in the already checked in simple thermostat, so it's a bit more inline stylistically with rest of files under `./samples/pnp`.

* ESP pipeline fixes (#1588)

* Update arduino-esp.yml for Azure Pipelines

* Update esp_arduino_uart_interface.py

* Update serial_connect.py

* Delete mxchip_settings.py

* try save env variable

* ok ok ok

* Update azure_test_firmware_errors.py

* release_2020_07_19_after_bump_version

* Update Long Term Support information

* Fix strftime format string (#1593)

Fix format string passed to strftime to use desired format. Matches what was already in the more complex component sample and embedded C samples.

* Fix incorrect handle name in telemetry sample (#1599)

* Add DPS sample code to temperature controller and helpers (#1597)

Adds DPS support to PnP temperature controller sample.

* Remove redundant helpers name from file and function names (#1605)

Naming functions and file name with `helper` wasn't particularly helpful.  Cleaning up.

* PnP sample refinements (#1607)

Further cleanup of PNP samples, most importantly returning an empty JSON blob on commands (hub/explorer aren't happy otherwise the way C SDK works).

Co-authored-by: Rajeev Massand <[email protected]>
Co-authored-by: Marcos Mokarzel <[email protected]>
Co-authored-by: John Spaith <[email protected]>
Co-authored-by: Ewerton <[email protected]>
Co-authored-by: jbobotek <[email protected]>
Co-authored-by: Jasmine Lo <[email protected]>
Co-authored-by: Robledo Pontes <[email protected]>
Co-authored-by: danewalton <[email protected]>
Co-authored-by: Luochao Wang <[email protected]>
Co-authored-by: Cristian Pop <[email protected]>
Co-authored-by: hihigupt <[email protected]>
Co-authored-by: az-iot-builder-01 <[email protected]>
Co-authored-by: Nicole Berdy <[email protected]>
Co-authored-by: Greg Manyak <[email protected]>
Co-authored-by: YAMAMOTO Takashi <[email protected]>
Co-authored-by: Spencer McDonough <[email protected]>
Co-authored-by: bikamani <[email protected]>
Co-authored-by: Rajasekharan Vengalil <[email protected]>
Co-authored-by: ksaye <[email protected]>
Co-authored-by: Rene Beckmann <[email protected]>
Co-authored-by: Eric Wolz <[email protected]>
Co-authored-by: Tye Tinsley <[email protected]>
Co-authored-by: ttins <[email protected]>
Co-authored-by: Wesley McSwain <[email protected]>
Co-authored-by: Kelly Gremban <[email protected]>
Co-authored-by: betarho <[email protected]>
Co-authored-by: Elena Horton <[email protected]>
ewertons added a commit that referenced this pull request Nov 13, 2020
* sendPendingGetTwinRequests only after the suback is received

* Update unit tests for fix on MQTT transport/sendPendingGetTwinRequests

Co-authored-by: Ewerton Scaboro da Silva <[email protected]>

Port db4875c into lts_02_2020.
CIPop pushed a commit that referenced this pull request Dec 9, 2020
* sendPendingGetTwinRequests only after the suback is received

* Update unit tests for fix on MQTT transport/sendPendingGetTwinRequests

Co-authored-by: Ewerton Scaboro da Silva <[email protected]>

Port db4875c into lts_02_2020.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants