Adding Rotating Device Id to BLE & DNS-SD discovery #4210

hnnajh · 2020-12-15T13:51:53Z

Problem

We need to integrate the Rotating Device Id with commissioning protocols (BLE/DNS-SD)

Summary of Changes

This PR adds the rotating device id to both BLE GATT characteristics and DNS-SD discovery as per specs + adds parsing command to parse the additional data payload coming over those two procotols

merge from master repo

Update from master

#### Problem Some conversions to use PacketBufferHandle (project-chip#3909) broke Darwin builds, which aren't currently run in CI. #### Summary of Changes Fix src/platform/Darwin/BleConnectionDelegateImpl.mm to match the API change in project-chip#3909.

* New seekbar in Android CHIPTool * Sample usage in lighting-app/nrfconnect Signed-off-by: Markus Becker <markus.becker@tridonic.com>

PR project-chip#3704 introduced a change that the BLE transport in RendezvousSession is only initialized when PeerAddress in RendezvousParams is of type BLE. However, PeerAddress isn't initialized anywhere. As a result Rendezvous over BLE stopped working between Android CHIPTool and accessories. Btw, remove an assert related to the storage delegate as it seems an optional member of the device controller.

* Fix data loss or crash in TCPEndPoint with LwIP #### Problem Under the configuration CHIP_SYSTEM_CONFIG_USE_LWIP, in some cases where the data size exceeds the TCP window size, TCPEndPoint can either die or lose data when accounting of un-acked data falls out of sync. #### Summary of Changes Imported fix from Weave: * This change removes separate accounting of the unsent data position and replaces it with simple counting of sent-but-not-acked data and a skip-loop at the start of DriveSending(). Fixes project-chip#4013 - Data loss or crash in TCPEndPoint with LwIP * Restyled by clang-format Co-authored-by: Restyled.io <commits@restyled.io>

…hip#4010)

)

…#3966) mDNS doesn't build with no device layer. Remove it from the build and add back the coverage that would catch this that was lost in project-chip#3340.

…ct-chip#3963)

Looking to remove usage of 'Utils' unless we have really no choice. 'UnitTestRegistration' seems clearer in what it does compared to 'TestUtils'.

…p#3991)

* Cleanup zap chip-helper.js * Restyled by clang-format Co-authored-by: Restyled.io <commits@restyled.io>

…#3999) * Move src/transport/tests to auto-test-driver generation * Add relevant libraries (and more test-capable libs) to nrf. * Refactor inet test helpers (to not include actual inet tests), try to make qemu allow better linkage but still failed for transport tests so disabled for now * Added more tests on esp32 qemu * Restyle fixes * Fix cast errors in InetCommon * Disable raw tests from zephyr: somehow they fail running out of endpoints * Disable DNS test on zephyr * Remove inet endpoint test from zephyr * Remove inet endpoint test from zephyr - fix again * Modify gitignore * Restyle fixes

…roject-chip#3979)

…ject-chip#4017) * Implement the missing part of Exchange Header in Transport layer * Revert comment 'if' back to 'iff'("if and only if")

project-chip#3994) * Remove duplicated send flag defines and put ExchangeMgr/ExchangeContext under the same namespace as CRMP * Rename kSendFlag_Default to kSendFlag_None

…ion (project-chip#3998) * Move src/lib/asn1/tests and src/ble/tests to auto-test-driver generation * Remove one more unused file * Attempt to enable asn1 and ble tests in esp32 - see if they pass or not * Fix merge error * Update include header for ASN1 test * Include ASN1 in libCHIP

* Some conversions to use PacketBufferHandle #### Problem Code should use `PacketBufferHandle` rather than `PacketBuffer *`. #### Summary of Changes - Converts remaining receive path in //src/inet and //src/transport. - Converts most of //src/ble. - Introduces Handle versions of the `AddToEnd`/`DetachTail` pair. Part of issue project-chip#2707 - Figure out a way to express PacketBuffer ownership in the type system * Restyled by clang-format * review * revive BtpEngine::Clear[TR]xPacket() * simplify conditional * (void) message.DetachTail() → message.FreeHead() * remove ExchangeContext::kSendFlag_RetainBuffer * missed pBuf.IsNull() * DetachHead() → PopTail() * typos Co-authored-by: Restyled.io <commits@restyled.io>

* Move src/system/tests to auto-test-driver generation * Remove a TCP/IP init call that was killing qemu

The "all" target exists implicitly and contains everything. We don't need to specify one, and it's misleading to do so specifying deps has no effect.

…4032)

bzbarsky-apple · 2020-12-17T22:02:25Z

examples/chip-tool/commands/payload/AdditionalDataParseCommand.cpp

+    for (size_t i = 0; i < len; i += 2)
+    {
+        auto str  = payloadString.substr(i, 2);
+        uint8_t x = (uint8_t) stoi(str, 0, 16);


So if the string is not hex digits this will throw, right? Probably OK, for chip-tool....

This file needs to #include <string>

that's strange, it's compiling on macos without including the string header , i added it

It's possible it included it indirectly via one of the other headers, but we shouldn't rely on that.

bzbarsky-apple · 2020-12-17T22:06:02Z

src/platform/Linux/CHIPBluezHelper.cpp

@@ -48,9 +48,11 @@
 *          Provides Bluez dbus implementatioon for BLE
 */

+#include <AdditionalDataPayload.h>


I'd think this would be #include <setup_payload/AdditionalDataPayload.h>.

i thought the same at first, but it turns out the sources in build.gn is including all the files under setup_payload
i compiled it on linux for further verification

Yes, but we shouldn't be doing that, even if we are right now. Should be including things rooted at src

I agree, i see this problem in alot of build files, i added an issue to track this problem, #4257

bzbarsky-apple · 2020-12-17T22:16:25Z

src/platform/Linux/CHIPBluezHelper.cpp

+
+    writer.Init(buffer);
+
+    err = writer.OpenContainer(ProfileTag(kProtocol_ServiceProvisioning, kTag_AdditionalDataExensionDescriptor), kTLVType_Structure,


Something here should set writer.ImplicitProfileId to get implicit profile tag encoding like the spec says.

Also, the tag (kProtocol_ServiceProvisioning, 0) is already defined to be kTag_QRCodeExensionDescriptor as far as I can tell. So this needs to use either a different profile or a different tag value (and the spec should be fixed accordingly).

i don't see a problem with using the same tag/profile here as well, those are two different payloads that will never be combined.

The whole point of profiles and tags in TLV is that a fully qualified tag uniquely identifies what you are working with.

If this payload is always known to be "additional data", we should just say that (based on the context it appears in) and use an anonymous tag for the outermost struct; that will save a few bytes on the wire too. The point of using a profile-based tag for the outermost container is if there is not enough context to tell what the thing is, so the tag is needed to disambiguate.

I got it, i create an issue #4259 , meanwhile, i will use anonymous tag for the outer most tag because it seems like the most suitable solution for now.

bzbarsky-apple · 2020-12-17T22:20:37Z

src/platform/Linux/CHIPBluezHelper.cpp

+    SuccessOrExit(err);
+
+    // Adding the rotating device id to the TLV data
+    err = innerWriter.PutString(ProfileTag(kProtocol_ServiceProvisioning, kRotatingDeviceIdTag), CHIP_ROTATING_DEVICE_ID);


Does not match the spec. Spec says to use ContextTag(0), whereas this is using a profile-specific tag with tag value 1.

bzbarsky-apple · 2020-12-17T22:23:04Z

src/setup_payload/AdditionalDataPayload.h

+namespace chip {
+
+constexpr uint16_t kRotatingDeviceIdLength               = 256;
+constexpr uint8_t kRotatingDeviceIdTag                   = 0x01;


Still has the wrong value.

bzbarsky-apple · 2020-12-17T22:26:14Z

src/setup_payload/AdditionalDataPayloadParser.cpp

+    chip::TLV::TLVReader reader;
+    chip::TLV::TLVReader innerReader;
+
+    reader.Init(mPayload.data(), (uint32_t) mPayload.size());


This is a bit sub-optimal because in practice the consumer of this class may not have the data in a vector. They will just have a byte buffer they got. They can copy into a vector, but forcing them to do that is not great.

Why not just have a static method, instead of a member storing the data. The static method takes a pointer to data, a length, and the AdditionalDataPayload reference. Callers that have a vector can use that memory; callers who have some other byte buffer can use that.

fair point, updated
but i kept the local populatePayload to be consistent with other parsers

bzbarsky-apple · 2020-12-17T22:27:14Z

src/setup_payload/AdditionalDataPayloadParser.cpp

+    err                      = reader.Next();
+    SuccessOrExit(err);
+
+    // Open the container


We need either code here to handle the error case or a code issue (mentioning the spec issue) tracking that code existing.

bzbarsky-apple · 2020-12-17T22:29:15Z

src/setup_payload/AdditionalDataPayloadParser.cpp

+    SuccessOrExit(err);
+
+    // Get the value of the rotating device id
+    if (chip::TLV::TagNumFromTag(innerReader.GetTag()) == kRotatingDeviceIdTag)


This could be true for all sorts of tags that are not what we want (e.g. various profile tags).

The check we actually want here, I think, is innerReader.GetTag() == ContextTag(kRotatingDeviceIdTag) or so.

bzbarsky-apple · 2020-12-17T22:31:23Z

src/setup_payload/AdditionalDataPayloadParser.cpp

+        err                         = innerReader.GetString(rotatingDeviceId, sizeof(rotatingDeviceId));
+        outPayload.rotatingDeviceId = std::string(rotatingDeviceId);


If the incoming size is too big, this will read completely random data until it hits a null or crashes. Probably need to SuccessOrExit(err) before examining rotatingDeviceId

good point, updated

bzbarsky-apple · 2020-12-17T22:31:51Z

src/setup_payload/AdditionalDataPayloadParser.h

+class AdditionalDataPayloadParser
+{
+private:
+    std::vector<uint8_t> mPayload;


Need to include stdint, right?

it didn't complain while compiling it on macos...

Might be getting included indirectly, but we shouldn't rely on that.

mspang

Please revert change to third_party/pigweed/repo. The following commands should work:

git fetch https://github.com/project-chip/connectedhomeip.git
git checkout $(git merge-base HEAD FETCH_HEAD)  third_party/pigweed/repo
git commit -m 'Revert change to pigweed submodule'
git submodule update third_party/pigweed/repo

Submodule path 'third_party/pigweed/repo': checked out '5a4db08b7b1df9261f5e2491ccb4d0caa10e6bb3'

mspang · 2020-12-17T22:30:33Z

src/platform/BUILD.gn

@@ -44,6 +44,9 @@ if (chip_device_platform != "none") {

    # By pass provision and secure session
    chip_bypass_rendezvous = false
+
+    # Bypass including the additional data in the advertisement packets
+    chip_bypass_additional_data_advertising = false


Suggest chip_enable_additional_data_advertising = true instead?

bzbarsky-apple · 2020-12-18T02:33:38Z

src/setup_payload/AdditionalDataPayloadParser.cpp

+
+    reader.Init(mPayload.data(), (uint32_t) mPayload.size());
+    reader.ImplicitProfileId = kProtocol_ServiceProvisioning;
+    err                      = reader.Next();


Actually, the two-argument form of Next() would be better here in terms of verifying that we are looking at what we think we should be looking at.

good point, updated

bzbarsky-apple · 2020-12-18T02:34:37Z

src/setup_payload/AdditionalDataPayloadParser.cpp

+    err = reader.OpenContainer(innerReader);
+    SuccessOrExit(err);
+
+    err = innerReader.Next();


Again, 2-argument form.

Alternately, if we ever really do expect random other things here, we should probably keep calling Next() until we run out of data or find something we understand.

good point, updated

bzbarsky-apple · 2020-12-18T21:25:58Z

src/lib/mdns/DiscoveryManager.cpp

-        { "VP", nullptr, 0 },
-    };
-
+#if CHIP_ENABLE_ADDITIONAL_ADVERTISING


Suggested change

#if CHIP_ENABLE_ADDITIONAL_ADVERTISING

#if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING

and similar for the other places where this is used?

bzbarsky-apple · 2020-12-18T21:26:51Z

src/lib/support/logging/CHIPLogging.h

@@ -109,6 +109,7 @@ enum LogModule
    kLogModule_Shell,
    kLogModule_DeviceLayer,
    kLogModule_SetupPayload,
+    kLogModule_AdditionalDataPayload,


This part is still (again?) here...

i added it back because i needed to log inside of AdditionalDataPayloadParser.cpp but not anymore, removed it

bzbarsky-apple · 2020-12-18T21:33:17Z

src/setup_payload/AdditionalDataPayloadParser.cpp

+    err = innerReader.GetString(rotatingDeviceId, sizeof(rotatingDeviceId));
+    SuccessOrExit(err);
+    outPayload.rotatingDeviceId = std::string(rotatingDeviceId);
+


Should we check that we're now at the end of the container? See VerifyEndOfContainer(). Again, this depends on the semantics expected here; the spec needs to define the behavior and doesn't right now.

added a basic check and opened an issue to track this #4265

bzbarsky-apple · 2020-12-18T21:35:08Z

src/setup_payload/AdditionalDataPayloadParser.h

+    const uint32_t mPayloadBufferLength;
+
+public:
+    AdditionalDataPayloadParser(const uint8_t * payloadBufferData, const uint32_t payloadBufferLength) :


Needs to document the lifetime assumption: the data being pointer to needs to outlive this parser.

added some documentation for both methods

andy31415 · 2021-01-05T19:17:56Z

src/platform/Linux/CHIPBluezHelper.cpp

+                                     g_memdup(buffer->Start(), buffer->DataLength()));
+    bluez_gatt_characteristic1_set_value(characteristic, cValue);
+
+    return;


could we remove these returns here? that could make the code easier to read.

andy31415 · 2021-01-05T19:21:23Z

src/ble/BleLayer.cpp

@@ -163,6 +163,10 @@ const ChipBleUUID BleLayer::CHIP_BLE_CHAR_2_ID = { { // 18EE2EF5-263D-4559-959F-
                                                     0x18, 0xEE, 0x2E, 0xF5, 0x26, 0x3D, 0x45, 0x59, 0x95, 0x9F, 0x4F, 0x9C, 0x42,
                                                     0x9F, 0x9D, 0x12 } };

+const ChipBleUUID BleLayer::CHIP_BLE_CHAR_3_ID = { { // 64630238-8772-45F2-B87D-748A83218F04


These numeric-based characteristic constants are a bit ugly to follow.

I think we should try for better C++ naming, like kBleCharacteristicID_Writes, kBleCharacteristicID_Indications etc.

Here and below in the #define CHIP_PLAT_BLE_UUID_C1_STRING "18ee2ef5-263d-4559-959f-4f9c429f9d11" code.

andy31415 · 2021-01-05T19:23:02Z

src/ble/BleLayer.cpp

@@ -600,7 +604,7 @@ bool BleLayer::HandleSubscribeReceived(BLE_CONNECTION_OBJECT connObj, const Chip
        return false;
    }

-    if (UUIDsMatch(&CHIP_BLE_CHAR_2_ID, charId))
+    if (UUIDsMatch(&CHIP_BLE_CHAR_2_ID, charId) || UUIDsMatch(&CHIP_BLE_CHAR_3_ID, charId))


could we extract this comparison in a function?

I wonder what the compare checks for and why 2 and 3 are special.
Is this IsKnownNonWriteCharacteristic(charID) or IsReadableDataCharacteristic(charId) or something else?

bzbarsky-apple · 2021-01-05T19:27:32Z

src/setup_payload/AdditionalDataPayloadParser.h

+     * @param[in] payloadBufferData        The buffer data for the additional data payload,
+     *                                     it needs to outlive the lifetime of this parse.


Suggested change

* @param[in] payloadBufferData The buffer data for the additional data payload,

* it needs to outlive the lifetime of this parse.

* @param[in] payloadBufferData The data buffer for the additional data payload,

* This buffer needs to stay alive while this parser object exists.

bzbarsky-apple · 2021-01-05T19:27:46Z

src/setup_payload/AdditionalDataPayloadParser.h

+     *
+     * @param[in] payloadBufferData        The buffer data for the additional data payload,
+     *                                     it needs to outlive the lifetime of this parse.
+     * @param[in] payloadBufferLength      The buffer data length for the additional data payload.


Suggested change

* @param[in] payloadBufferLength The buffer data length for the additional data payload.

* @param[in] payloadBufferLength The data buffer length for the additional data payload.

rwalker-apple · 2021-01-05T21:44:10Z

would you mind terribly updating the PR description according to the template? Please state the

Problem

being solved and the

Summary of Changes

in this PR

mspang · 2021-01-05T21:11:22Z

src/platform/BUILD.gn

@@ -79,6 +82,12 @@ if (chip_device_platform != "none") {
      defines += [ "CHIP_BYPASS_RENDEZVOUS=1" ]
    }

+    if (chip_enable_additional_data_advertising) {


nit: prefer

defines += [ "CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING=${chip_enable_additional_data_advertising}" ]

mspang · 2021-01-05T21:13:07Z

src/setup_payload/AdditionalDataPayload.h

+
+struct AdditionalDataPayload
+{
+public:


nit: remove public

It's the default for structs.

* RotatingId: version0 * RotatingId: version1 * RotatingId: version0 * RotatingId: version1 * Fix Darwin host build (project-chip#3990) #### Problem Some conversions to use PacketBufferHandle (project-chip#3909) broke Darwin builds, which aren't currently run in CI. #### Summary of Changes Fix src/platform/Darwin/BleConnectionDelegateImpl.mm to match the API change in project-chip#3909. * Add '-Wextra' to compiler flags (project-chip#3902) * Implement Level Control Cluster (project-chip#3806) * New seekbar in Android CHIPTool * Sample usage in lighting-app/nrfconnect Signed-off-by: Markus Becker <markus.becker@tridonic.com> * Fix Rendezvous over BLE after recent changes (project-chip#4012) PR project-chip#3704 introduced a change that the BLE transport in RendezvousSession is only initialized when PeerAddress in RendezvousParams is of type BLE. However, PeerAddress isn't initialized anywhere. As a result Rendezvous over BLE stopped working between Android CHIPTool and accessories. Btw, remove an assert related to the storage delegate as it seems an optional member of the device controller. * [thread] fix invalid configuration of active dataset (project-chip#4008) * Fix data loss or crash in TCPEndPoint with LwIP (project-chip#4022) * Fix data loss or crash in TCPEndPoint with LwIP #### Problem Under the configuration CHIP_SYSTEM_CONFIG_USE_LWIP, in some cases where the data size exceeds the TCP window size, TCPEndPoint can either die or lose data when accounting of un-acked data falls out of sync. #### Summary of Changes Imported fix from Weave: * This change removes separate accounting of the unsent data position and replaces it with simple counting of sent-but-not-acked data and a skip-loop at the start of DriveSending(). Fixes project-chip#4013 - Data loss or crash in TCPEndPoint with LwIP * Restyled by clang-format Co-authored-by: Restyled.io <commits@restyled.io> * Update lighting-app gen/ folder with ZAP generated content (project-chip#4010) * Fix segmentation fault error in response echo message (project-chip#3984) * Add back Android default build coverage & fix the build (project-chip#3966) mDNS doesn't build with no device layer. Remove it from the build and add back the coverage that would catch this that was lost in project-chip#3340. * Update all-clusters-app gen/ folder with ZAP generated content (project-chip#3963) * Move src/inet/tests to auto-test-driver generation (project-chip#3997) * Rename TestUtils to UnitTestRegistration. (project-chip#4021) Looking to remove usage of 'Utils' unless we have really no choice. 'UnitTestRegistration' seems clearer in what it does compared to 'TestUtils'. * Update src/lib/core/tests to auto-test-driver generation (project-chip#3991) * Cleanup zap chip-helper.js (project-chip#3973) * Cleanup zap chip-helper.js * Restyled by clang-format Co-authored-by: Restyled.io <commits@restyled.io> * Move src/transport/tests to auto-test-driver generation (project-chip#3999) * Move src/transport/tests to auto-test-driver generation * Add relevant libraries (and more test-capable libs) to nrf. * Refactor inet test helpers (to not include actual inet tests), try to make qemu allow better linkage but still failed for transport tests so disabled for now * Added more tests on esp32 qemu * Restyle fixes * Fix cast errors in InetCommon * Disable raw tests from zephyr: somehow they fail running out of endpoints * Disable DNS test on zephyr * Remove inet endpoint test from zephyr * Remove inet endpoint test from zephyr - fix again * Modify gitignore * Restyle fixes * Use CHIPDeviceController instead of CHIPDeviceController_deprecated (project-chip#3979) * Implement the missing part of Exchange Header in Transport layer (project-chip#4017) * Implement the missing part of Exchange Header in Transport layer * Revert comment 'if' back to 'iff'("if and only if") * Remove duplicated send flag defines and put ExchangeMgr/ExchangeConte… (project-chip#3994) * Remove duplicated send flag defines and put ExchangeMgr/ExchangeContext under the same namespace as CRMP * Rename kSendFlag_Default to kSendFlag_None * Move src/lib/asn1/tests and src/ble/tests to auto-test-driver generation (project-chip#3998) * Move src/lib/asn1/tests and src/ble/tests to auto-test-driver generation * Remove one more unused file * Attempt to enable asn1 and ble tests in esp32 - see if they pass or not * Fix merge error * Update include header for ASN1 test * Include ASN1 in libCHIP * Some conversions to use PacketBufferHandle (project-chip#4011) * Some conversions to use PacketBufferHandle #### Problem Code should use `PacketBufferHandle` rather than `PacketBuffer *`. #### Summary of Changes - Converts remaining receive path in //src/inet and //src/transport. - Converts most of //src/ble. - Introduces Handle versions of the `AddToEnd`/`DetachTail` pair. Part of issue project-chip#2707 - Figure out a way to express PacketBuffer ownership in the type system * Restyled by clang-format * review * revive BtpEngine::Clear[TR]xPacket() * simplify conditional * (void) message.DetachTail() → message.FreeHead() * remove ExchangeContext::kSendFlag_RetainBuffer * missed pBuf.IsNull() * DetachHead() → PopTail() * typos Co-authored-by: Restyled.io <commits@restyled.io> * Move src/system/tests to auto-test-driver generation (project-chip#4000) * Move src/system/tests to auto-test-driver generation * Remove a TCP/IP init call that was killing qemu * Remove explicit "all" target from root build file (project-chip#3967) The "all" target exists implicitly and contains everything. We don't need to specify one, and it's misleading to do so specifying deps has no effect. * Make src/setup_payload compile with -Werror=conversion (project-chip#4032) * Add SSID and password to chip-tool pairing (project-chip#4054) * Get temperature-measurement and all-clusters-app to use examples/common/chip-app-server (project-chip#4039) #### Problem PR project-chip#3704 introduced a change where a `PeerAddress` is now required in order to start `RendezvousSession`. Sadly the multiple code paths bootstrapping `RendezvousSession` has not been updated. PR project-chip#4012 add a fix for some of the `examples/` but not for the `all-clusters-app` nor the `temperature-measurement-app`. To avoid such situation, this PR merge `examples/common/chip-app-server` and the custom code from `all-clusters-app` and `temperature-measurement-app`. One of the more discutable change of this PR (imo) is the code that moves the custom `Echo` mechanism from the `all-clusters-app` to `chip-app-server`. I was hoping to get rid of it before doing this change but the `all-clusters-app` and the `temperature-measurement-app` are broken since project-chip#3704 and this PR should fix that. Also I have a PR (mostly) ready once project-chip#3979 lands to get rid of this `Echo` specific code and replace it by a manufacturer specific `ping` command. #### Summary of Changes * Remove `EchoServer.cpp`, `RendezvousDeviceDelegate.cpp` and `include/RendezvousDeviceDelegate.h` from `all-clusters-app` * Remove `ResponseServer.cpp`, `RendezvousDeviceDelegate.cpp` and `include/RendezvousDeviceDelegate.h` from `temperature-measurement-app` * Introduce `chip-app-server/include/AppDelegate.h` in order to keep the behavior the `all-clusters-app` that turns on/off leds on different events. Maybe it should be converted to some types of `ChipDeviceEvent` or `CHIPCallback` at some point. * Fix `chip-app-server` to accomodate for the specifics of `all-clusters-app` * Add all Thread ULA addresses to the lwip interface (project-chip#4053) ULA prefixes will used for CHIP network so we need to add all these addresses to the interface. * Remove src/lib/message. (project-chip#4055) * Remove src/lib/message. Updated messaging implementation lives in src/messaging and the src/lib/message is not currently compiled or linked in. This saves us from looking at this code when some refactoring is needed (e.g. the SystemPacketBuffer changes). * Remove one more unused file * [ChipTool] Add Payload Parse Command (project-chip#3696) * [ChipTool] Add Payload Parse Command * [ChipTool] Add Payload Parse Command * [ChipTool] Restyle issues resolved * Restyled by whitespace * Resolve build errors caused by Command.h schema change Co-authored-by: lijayaku <lijayaku@amazon.com> Co-authored-by: Restyled.io <commits@restyled.io> * rename ParseCommand to QRCodeParseCommand * adding AdditionalDataParseCommand version 0 * Adding parsing logic + logging * adding another parsing method * Basic Parsing is DONE * fixing memory issue * removing some logs * removing more logs * minor update * Add RotatingDeviceId to DNS-SD * fix compilation problem * fix minor diffs * cleanup rotating id in ble * fix dns * nits * fix compilation * Revert "fix minor diffs" This reverts commit 88fb69c. * nits * nit fixes * update allocation * update allocation * refactoring * revert settings.json * fix styling * Update README file * adding a build flag to bypass advertising the additional data field * fix styling * fixing README style * Fixing nits + added description about the additional data * fixed minor comment * remove ParseCommand * removing unused headers + nits * Fixing Additional data TLV Tags * Fix Styling * removed unnecessary logging tag * fixing writing rotating id * fixing minor styles * adding check for rotating id tag * restyling * update AdditionalDataPayloadParser to work on vector of bytes * restyling * Nit fix * Renaming QRCode command to SetupPayload command * update parse command for additional data * change compile flags * update some comments * fixing parsing/generating the additional data payload * changing the additional parser APIs * restyling * reverting pigweed repo changes * rename CHIP_ENABLE_ADDITIONAL_ADVERTISING * removing logging tag * adding extra validation in parser * adding docs to the additional data payload parser * Update ReadME.md Co-authored-by: Kevin Schoedel <67607049+kpschoedel@users.noreply.github.com> Co-authored-by: Vivien Nicolas <vnicolas@apple.com> Co-authored-by: Markus Becker <Markus.Becker@tridonic.com> Co-authored-by: Damian Królik <66667989+Damian-Nordic@users.noreply.github.com> Co-authored-by: Łukasz Duda <lukasz.duda@nordicsemi.no> Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Yufeng Wang <44623591+yufengwangca@users.noreply.github.com> Co-authored-by: Michael Spang <spang@google.com> Co-authored-by: Andrei Litvin <andrei@andy314.com> Co-authored-by: jepenven-silabs <67962328+jepenven-silabs@users.noreply.github.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Jiacheng Guo <gjc@google.com> Co-authored-by: Liju Jayakumar <26148162+lijujayakumar@users.noreply.github.com> Co-authored-by: lijayaku <lijayaku@amazon.com>

hnnajh and others added 30 commits November 25, 2020 08:02

Merge pull request #1 from project-chip/master

5ad2124

merge from master repo

RotatingId: version0

2da216b

RotatingId: version1

d7be05e

Merge pull request #2 from project-chip/master

88bc2a4

Update from master

RotatingId: version0

ceffd58

RotatingId: version1

4b6f8e1

Add '-Wextra' to compiler flags (project-chip#3902)

9ca3ff1

Implement Level Control Cluster (project-chip#3806)

2f053df

* New seekbar in Android CHIPTool * Sample usage in lighting-app/nrfconnect Signed-off-by: Markus Becker <markus.becker@tridonic.com>

[thread] fix invalid configuration of active dataset (project-chip#4008)

5563c31

Update lighting-app gen/ folder with ZAP generated content (project-c…

f7e0ea1

…hip#4010)

Fix segmentation fault error in response echo message (project-chip#3984

1d731f0

)

Add back Android default build coverage & fix the build (project-chip…

a23bfef

…#3966) mDNS doesn't build with no device layer. Remove it from the build and add back the coverage that would catch this that was lost in project-chip#3340.

Update all-clusters-app gen/ folder with ZAP generated content (proje…

ade5ac5

…ct-chip#3963)

Move src/inet/tests to auto-test-driver generation (project-chip#3997)

eea78f6

Rename TestUtils to UnitTestRegistration. (project-chip#4021)

307a5e8

Looking to remove usage of 'Utils' unless we have really no choice. 'UnitTestRegistration' seems clearer in what it does compared to 'TestUtils'.

Update src/lib/core/tests to auto-test-driver generation (project-chi…

2c9a83e

…p#3991)

Cleanup zap chip-helper.js (project-chip#3973)

b6e404b

* Cleanup zap chip-helper.js * Restyled by clang-format Co-authored-by: Restyled.io <commits@restyled.io>

Use CHIPDeviceController instead of CHIPDeviceController_deprecated (p…

abc5d4f

…roject-chip#3979)

Implement the missing part of Exchange Header in Transport layer (pro…

449afa8

…ject-chip#4017) * Implement the missing part of Exchange Header in Transport layer * Revert comment 'if' back to 'iff'("if and only if")

Remove duplicated send flag defines and put ExchangeMgr/ExchangeConte… (

e1ba0f5

project-chip#3994) * Remove duplicated send flag defines and put ExchangeMgr/ExchangeContext under the same namespace as CRMP * Rename kSendFlag_Default to kSendFlag_None

Move src/system/tests to auto-test-driver generation (project-chip#4000)

7808bda

* Move src/system/tests to auto-test-driver generation * Remove a TCP/IP init call that was killing qemu

Remove explicit "all" target from root build file (project-chip#3967)

d76cb70

The "all" target exists implicitly and contains everything. We don't need to specify one, and it's misleading to do so specifying deps has no effect.

Make src/setup_payload compile with -Werror=conversion (project-chip#…

1d3dca5

…4032)

Add SSID and password to chip-tool pairing (project-chip#4054)

6c8a429

Nit fix

0438185

hnnajh requested a review from bzbarsky-apple December 17, 2020 20:42

hnnajh added 2 commits December 17, 2020 13:17

Renaming QRCode command to SetupPayload command

a591ed3

update parse command for additional data

e60d23e

hnnajh requested a review from lijujayakumar December 17, 2020 22:13

bzbarsky-apple requested changes Dec 17, 2020

View reviewed changes

mspang requested changes Dec 17, 2020

View reviewed changes

hnnajh added 2 commits December 17, 2020 16:32

change compile flags

5d13ea6

update some comments

b697928

restyled-io bot mentioned this pull request Dec 18, 2020

Restyle Adding Rotating Device Id to BLE & DNS-SD discovery #4251

Closed

bzbarsky-apple reviewed Dec 18, 2020

View reviewed changes

hnnajh added 3 commits December 18, 2020 12:20

fixing parsing/generating the additional data payload

63501cd

changing the additional parser APIs

f66d76f

restyling

9f011ec

bzbarsky-apple reviewed Dec 18, 2020

View reviewed changes

hnnajh added 5 commits December 18, 2020 16:20

reverting pigweed repo changes

df8ca0b

rename CHIP_ENABLE_ADDITIONAL_ADVERTISING

65fa927

removing logging tag

8dce5e2

adding extra validation in parser

81b3bde

adding docs to the additional data payload parser

3fad97e

hnnajh requested review from mspang and bzbarsky-apple January 5, 2021 16:23

Update ReadME.md

11aca36

andy31415 approved these changes Jan 5, 2021

View reviewed changes

bzbarsky-apple approved these changes Jan 5, 2021

View reviewed changes

mspang approved these changes Jan 6, 2021

View reviewed changes

chrisdecenzo approved these changes Jan 6, 2021

View reviewed changes

chrisdecenzo merged commit 9661374 into project-chip:master Jan 6, 2021


		writer.Init(buffer);

		err = writer.OpenContainer(ProfileTag(kProtocol_ServiceProvisioning, kTag_AdditionalDataExensionDescriptor), kTLVType_Structure,

		err = innerReader.GetString(rotatingDeviceId, sizeof(rotatingDeviceId));
		outPayload.rotatingDeviceId = std::string(rotatingDeviceId);

	#if CHIP_ENABLE_ADDITIONAL_ADVERTISING
	#if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING

		* @param[in] payloadBufferData The buffer data for the additional data payload,
		* it needs to outlive the lifetime of this parse.

	* @param[in] payloadBufferLength The buffer data length for the additional data payload.
	* @param[in] payloadBufferLength The data buffer length for the additional data payload.

Adding Rotating Device Id to BLE & DNS-SD discovery #4210

Adding Rotating Device Id to BLE & DNS-SD discovery #4210

Conversation

hnnajh commented Dec 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mspang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwalker-apple commented Jan 5, 2021

Problem

Summary of Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hnnajh commented Dec 15, 2020 •

edited

Loading