From 003c478222211e877755ae2e0f58a4779a06bc8c Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Fri, 23 Feb 2024 10:54:21 +0100 Subject: [PATCH 1/3] [Linux] Fix memory leaks when handling BluezAdapter1 objects (#32263) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Convert glib object to GAutoPtr<> in Bluez * Do not keep a copy of BluezAdapter1 properties * Use GAutoPtr to manage BluezAdapter1 life cycle * Simplify adapter retrieval from BlueZ object * Properly release objects obtained with bluez_object_get_*() * Use reinterpret_cast when using g_object_ref() * Use on-the-stack strings to simplify memory management * Manage BluezLEAdvertisement1 with GAutoPtr * Do not leak GDBusObjectManagerServer in case of re-initialization * Fix life cycle of exported advertisement D-Bus object * Restart BLE advertisement in case of premature release --------- Co-authored-by: Damian Michalak-Szmaciński --- src/platform/GLibTypeDeleter.h | 6 + src/platform/Linux/BLEManagerImpl.cpp | 30 +++-- src/platform/Linux/BLEManagerImpl.h | 7 +- src/platform/Linux/CHIPDevicePlatformEvent.h | 11 +- src/platform/Linux/bluez/AdapterIterator.cpp | 47 +++---- src/platform/Linux/bluez/AdapterIterator.h | 28 ++-- .../Linux/bluez/BluezAdvertisement.cpp | 123 +++++++----------- src/platform/Linux/bluez/BluezAdvertisement.h | 14 +- src/platform/Linux/bluez/BluezConnection.cpp | 16 +-- src/platform/Linux/bluez/BluezEndpoint.cpp | 70 +++++----- src/platform/Linux/bluez/BluezEndpoint.h | 4 +- .../Linux/bluez/ChipDeviceScanner.cpp | 18 +-- src/platform/Linux/bluez/ChipDeviceScanner.h | 6 +- src/platform/Linux/bluez/Types.h | 24 ++++ 14 files changed, 194 insertions(+), 210 deletions(-) diff --git a/src/platform/GLibTypeDeleter.h b/src/platform/GLibTypeDeleter.h index f217d559e9cc66..f083a6c5e460d5 100644 --- a/src/platform/GLibTypeDeleter.h +++ b/src/platform/GLibTypeDeleter.h @@ -120,6 +120,12 @@ struct GAutoPtrDeleter using deleter = GObjectDeleter; }; +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + template <> struct GAutoPtrDeleter { diff --git a/src/platform/Linux/BLEManagerImpl.cpp b/src/platform/Linux/BLEManagerImpl.cpp index d5129a983f092d..dbf3791c905ac1 100644 --- a/src/platform/Linux/BLEManagerImpl.cpp +++ b/src/platform/Linux/BLEManagerImpl.cpp @@ -289,8 +289,13 @@ void BLEManagerImpl::HandlePlatformSpecificBLEEvent(const ChipDeviceEvent * apEv case DeviceEventType::kPlatformLinuxBLEPeripheralAdvStartComplete: VerifyOrExit(apEvent->Platform.BLEPeripheralAdvStartComplete.mIsSuccess, err = CHIP_ERROR_INCORRECT_STATE); sInstance.mFlags.Clear(Flags::kControlOpInProgress).Clear(Flags::kAdvertisingRefreshNeeded); - // Start a timer to make sure that the fast advertising is stopped after specified timeout. - SuccessOrExit(err = DeviceLayer::SystemLayer().StartTimer(kFastAdvertiseTimeout, HandleAdvertisingTimer, this)); + // Do not restart the timer if it is still active. This is to avoid the timer from being restarted + // if the advertising is stopped due to a premature release. + if (!DeviceLayer::SystemLayer().IsTimerActive(HandleAdvertisingTimer, this)) + { + // Start a timer to make sure that the fast advertising is stopped after specified timeout. + SuccessOrExit(err = DeviceLayer::SystemLayer().StartTimer(kFastAdvertiseTimeout, HandleAdvertisingTimer, this)); + } sInstance.mFlags.Set(Flags::kAdvertising); break; case DeviceEventType::kPlatformLinuxBLEPeripheralAdvStopComplete: @@ -305,6 +310,11 @@ void BLEManagerImpl::HandlePlatformSpecificBLEEvent(const ChipDeviceEvent * apEv ChipLogProgress(DeviceLayer, "CHIPoBLE advertising stopped"); } break; + case DeviceEventType::kPlatformLinuxBLEPeripheralAdvReleased: + // If the advertising was stopped due to a premature release, check if it needs to be restarted. + sInstance.mFlags.Clear(Flags::kAdvertising); + DriveBLEState(); + break; case DeviceEventType::kPlatformLinuxBLEPeripheralRegisterAppComplete: VerifyOrExit(apEvent->Platform.BLEPeripheralRegisterAppComplete.mIsSuccess, err = CHIP_ERROR_INCORRECT_STATE); mFlags.Set(Flags::kAppRegistered); @@ -780,30 +790,34 @@ CHIP_ERROR BLEManagerImpl::CancelConnection() return CHIP_NO_ERROR; } -void BLEManagerImpl::NotifyBLEPeripheralRegisterAppComplete(bool aIsSuccess, void * apAppstate) +void BLEManagerImpl::NotifyBLEPeripheralRegisterAppComplete(bool aIsSuccess) { ChipDeviceEvent event; event.Type = DeviceEventType::kPlatformLinuxBLEPeripheralRegisterAppComplete; event.Platform.BLEPeripheralRegisterAppComplete.mIsSuccess = aIsSuccess; - event.Platform.BLEPeripheralRegisterAppComplete.mpAppstate = apAppstate; PlatformMgr().PostEventOrDie(&event); } -void BLEManagerImpl::NotifyBLEPeripheralAdvStartComplete(bool aIsSuccess, void * apAppstate) +void BLEManagerImpl::NotifyBLEPeripheralAdvStartComplete(bool aIsSuccess) { ChipDeviceEvent event; event.Type = DeviceEventType::kPlatformLinuxBLEPeripheralAdvStartComplete; event.Platform.BLEPeripheralAdvStartComplete.mIsSuccess = aIsSuccess; - event.Platform.BLEPeripheralAdvStartComplete.mpAppstate = apAppstate; PlatformMgr().PostEventOrDie(&event); } -void BLEManagerImpl::NotifyBLEPeripheralAdvStopComplete(bool aIsSuccess, void * apAppstate) +void BLEManagerImpl::NotifyBLEPeripheralAdvStopComplete(bool aIsSuccess) { ChipDeviceEvent event; event.Type = DeviceEventType::kPlatformLinuxBLEPeripheralAdvStopComplete; event.Platform.BLEPeripheralAdvStopComplete.mIsSuccess = aIsSuccess; - event.Platform.BLEPeripheralAdvStopComplete.mpAppstate = apAppstate; + PlatformMgr().PostEventOrDie(&event); +} + +void BLEManagerImpl::NotifyBLEPeripheralAdvReleased() +{ + ChipDeviceEvent event; + event.Type = DeviceEventType::kPlatformLinuxBLEPeripheralAdvReleased; PlatformMgr().PostEventOrDie(&event); } diff --git a/src/platform/Linux/BLEManagerImpl.h b/src/platform/Linux/BLEManagerImpl.h index 61d2dff02757c7..6e3c1a687a5594 100644 --- a/src/platform/Linux/BLEManagerImpl.h +++ b/src/platform/Linux/BLEManagerImpl.h @@ -92,9 +92,10 @@ class BLEManagerImpl final : public BLEManager, static void HandleTXCharCCCDWrite(BLE_CONNECTION_OBJECT user_data); static void HandleTXComplete(BLE_CONNECTION_OBJECT user_data); - static void NotifyBLEPeripheralRegisterAppComplete(bool aIsSuccess, void * apAppstate); - static void NotifyBLEPeripheralAdvStartComplete(bool aIsSuccess, void * apAppstate); - static void NotifyBLEPeripheralAdvStopComplete(bool aIsSuccess, void * apAppstate); + static void NotifyBLEPeripheralRegisterAppComplete(bool aIsSuccess); + static void NotifyBLEPeripheralAdvStartComplete(bool aIsSuccess); + static void NotifyBLEPeripheralAdvStopComplete(bool aIsSuccess); + static void NotifyBLEPeripheralAdvReleased(); private: // ===== Members that implement the BLEManager internal interface. diff --git a/src/platform/Linux/CHIPDevicePlatformEvent.h b/src/platform/Linux/CHIPDevicePlatformEvent.h index 8618b9ebeaa0a8..b6e0b55cafdbb2 100644 --- a/src/platform/Linux/CHIPDevicePlatformEvent.h +++ b/src/platform/Linux/CHIPDevicePlatformEvent.h @@ -54,7 +54,8 @@ enum InternalPlatformSpecificEventTypes kPlatformLinuxBLEOutOfBuffersEvent, kPlatformLinuxBLEPeripheralRegisterAppComplete, kPlatformLinuxBLEPeripheralAdvStartComplete, - kPlatformLinuxBLEPeripheralAdvStopComplete + kPlatformLinuxBLEPeripheralAdvStopComplete, + kPlatformLinuxBLEPeripheralAdvReleased, }; } // namespace DeviceEventType @@ -91,22 +92,14 @@ struct ChipDevicePlatformEvent struct { bool mIsSuccess; - void * mpAppstate; } BLEPeripheralRegisterAppComplete; struct { bool mIsSuccess; - void * mpAppstate; - } BLEPeripheralAdvConfiguredComplete; - struct - { - bool mIsSuccess; - void * mpAppstate; } BLEPeripheralAdvStartComplete; struct { bool mIsSuccess; - void * mpAppstate; } BLEPeripheralAdvStopComplete; }; }; diff --git a/src/platform/Linux/bluez/AdapterIterator.cpp b/src/platform/Linux/bluez/AdapterIterator.cpp index e69c473f52bbfe..5510ceae996ce2 100644 --- a/src/platform/Linux/bluez/AdapterIterator.cpp +++ b/src/platform/Linux/bluez/AdapterIterator.cpp @@ -37,12 +37,6 @@ AdapterIterator::~AdapterIterator() { g_list_free_full(mObjectList, g_object_unref); } - - if (mCurrent.adapter != nullptr) - { - g_object_unref(mCurrent.adapter); - mCurrent.adapter = nullptr; - } } CHIP_ERROR AdapterIterator::Initialize(AdapterIterator * self) @@ -90,30 +84,7 @@ bool AdapterIterator::Advance() continue; } - // PATH is of the for BLUEZ_PATH / hci, i.e. like - // '/org/bluez/hci0' - // Index represents the number after hci - const char * path = g_dbus_proxy_get_object_path(G_DBUS_PROXY(adapter)); - unsigned index = 0; - - if (sscanf(path, BLUEZ_PATH "/hci%u", &index) != 1) - { - ChipLogError(DeviceLayer, "Failed to extract HCI index from '%s'", StringOrNullMarker(path)); - index = 0; - } - - if (mCurrent.adapter != nullptr) - { - g_object_unref(mCurrent.adapter); - mCurrent.adapter = nullptr; - } - - mCurrent.index = index; - mCurrent.address = bluez_adapter1_get_address(adapter); - mCurrent.alias = bluez_adapter1_get_alias(adapter); - mCurrent.name = bluez_adapter1_get_name(adapter); - mCurrent.powered = bluez_adapter1_get_powered(adapter); - mCurrent.adapter = adapter; + mCurrentAdapter.reset(adapter); mCurrentListItem = mCurrentListItem->next; @@ -123,6 +94,22 @@ bool AdapterIterator::Advance() return false; } +uint32_t AdapterIterator::GetIndex() const +{ + // PATH is of the for BLUEZ_PATH / hci, i.e. like '/org/bluez/hci0' + // Index represents the number after hci + const char * path = g_dbus_proxy_get_object_path(G_DBUS_PROXY(mCurrentAdapter.get())); + unsigned index = 0; + + if (sscanf(path, BLUEZ_PATH "/hci%u", &index) != 1) + { + ChipLogError(DeviceLayer, "Failed to extract HCI index from '%s'", StringOrNullMarker(path)); + index = 0; + } + + return index; +} + bool AdapterIterator::Next() { if (mManager == nullptr) diff --git a/src/platform/Linux/bluez/AdapterIterator.h b/src/platform/Linux/bluez/AdapterIterator.h index 85697b5d544c9a..0d44074889773b 100644 --- a/src/platform/Linux/bluez/AdapterIterator.h +++ b/src/platform/Linux/bluez/AdapterIterator.h @@ -56,12 +56,12 @@ class AdapterIterator // Information about the current value. Safe to call only after // "Next" has returned true. - uint32_t GetIndex() const { return mCurrent.index; } - const char * GetAddress() const { return mCurrent.address.c_str(); } - const char * GetAlias() const { return mCurrent.alias.c_str(); } - const char * GetName() const { return mCurrent.name.c_str(); } - bool IsPowered() const { return mCurrent.powered; } - BluezAdapter1 * GetAdapter() const { return mCurrent.adapter; } + uint32_t GetIndex() const; + const char * GetAddress() const { return bluez_adapter1_get_address(mCurrentAdapter.get()); } + const char * GetAlias() const { return bluez_adapter1_get_alias(mCurrentAdapter.get()); } + const char * GetName() const { return bluez_adapter1_get_name(mCurrentAdapter.get()); } + bool IsPowered() const { return bluez_adapter1_get_powered(mCurrentAdapter.get()); } + BluezAdapter1 * GetAdapter() const { return mCurrentAdapter.get(); } private: /// Sets up the DBUS manager and loads the list @@ -73,23 +73,11 @@ class AdapterIterator /// iterate through. bool Advance(); - static constexpr size_t kMaxAddressLength = 19; // xx:xx:xx:xx:xx:xx - static constexpr size_t kMaxNameLength = 64; - GDBusObjectManager * mManager = nullptr; // DBus connection GList * mObjectList = nullptr; // listing of objects on the bus GList * mCurrentListItem = nullptr; // current item viewed in the list - - // data valid only if Next() returns true - struct - { - uint32_t index; - std::string address; - std::string alias; - std::string name; - bool powered; - BluezAdapter1 * adapter; - } mCurrent = { 0 }; + // Data valid only if Next() returns true + GAutoPtr mCurrentAdapter; }; } // namespace Internal diff --git a/src/platform/Linux/bluez/BluezAdvertisement.cpp b/src/platform/Linux/bluez/BluezAdvertisement.cpp index 196529816b59ab..06f831bff8b58f 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.cpp +++ b/src/platform/Linux/bluez/BluezAdvertisement.cpp @@ -45,13 +45,13 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() GVariant * serviceUUID; GVariantBuilder serviceUUIDsBuilder; - ChipLogDetail(DeviceLayer, "Create BLE adv object at %s", mpAdvPath); - object = bluez_object_skeleton_new(mpAdvPath); + ChipLogDetail(DeviceLayer, "Create BLE adv object at %s", mAdvPath); + object = bluez_object_skeleton_new(mAdvPath); adv = bluez_leadvertisement1_skeleton_new(); g_variant_builder_init(&serviceUUIDsBuilder, G_VARIANT_TYPE("as")); - g_variant_builder_add(&serviceUUIDsBuilder, "s", mpAdvUUID); + g_variant_builder_add(&serviceUUIDsBuilder, "s", mAdvUUID); serviceUUID = g_variant_builder_end(&serviceUUIDsBuilder); @@ -81,7 +81,7 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() }), this); - g_dbus_object_manager_server_export(mpRoot, G_DBUS_OBJECT_SKELETON(object)); + g_dbus_object_manager_server_export(mRoot.get(), G_DBUS_OBJECT_SKELETON(object)); g_object_unref(object); return adv; @@ -89,11 +89,11 @@ BluezLEAdvertisement1 * BluezAdvertisement::CreateLEAdvertisement() gboolean BluezAdvertisement::BluezLEAdvertisement1Release(BluezLEAdvertisement1 * aAdv, GDBusMethodInvocation * aInvocation) { - ChipLogDetail(DeviceLayer, "Release BLE adv object in %s", __func__); - g_dbus_object_manager_server_unexport(mpRoot, mpAdvPath); - g_object_unref(mpAdv); - mpAdv = nullptr; + // This method is called when the advertisement is stopped (released) by BlueZ. + // We can use it to update the state of the advertisement in the CHIP layer. + ChipLogDetail(DeviceLayer, "BLE advertisement stopped by BlueZ"); mIsAdvertising = false; + BLEManagerImpl::NotifyBLEPeripheralAdvReleased(); return TRUE; } @@ -103,7 +103,7 @@ CHIP_ERROR BluezAdvertisement::InitImpl() // all D-Bus signals will be delivered to the GLib global default main context. VerifyOrDie(g_main_context_get_thread_default() != nullptr); - mpAdv = CreateLEAdvertisement(); + mAdv.reset(CreateLEAdvertisement()); return CHIP_NO_ERROR; } @@ -112,19 +112,19 @@ CHIP_ERROR BluezAdvertisement::Init(const BluezEndpoint & aEndpoint, const char GAutoPtr rootPath; CHIP_ERROR err; - VerifyOrExit(mpAdv == nullptr, err = CHIP_ERROR_INCORRECT_STATE; + VerifyOrExit(!mAdv, err = CHIP_ERROR_INCORRECT_STATE; ChipLogError(DeviceLayer, "FAIL: BLE advertisement already initialized in %s", __func__)); - mpRoot = reinterpret_cast(g_object_ref(aEndpoint.GetGattApplicationObjectManager())); - mpAdapter = reinterpret_cast(g_object_ref(aEndpoint.GetAdapter())); + mRoot.reset(reinterpret_cast(g_object_ref(aEndpoint.GetGattApplicationObjectManager()))); + mAdapter.reset(reinterpret_cast(g_object_ref(aEndpoint.GetAdapter()))); - g_object_get(G_OBJECT(mpRoot), "object-path", &rootPath.GetReceiver(), nullptr); - mpAdvPath = g_strdup_printf("%s/advertising", rootPath.get()); - mpAdvUUID = g_strdup(aAdvUUID); + g_object_get(G_OBJECT(mRoot.get()), "object-path", &rootPath.GetReceiver(), nullptr); + g_snprintf(mAdvPath, sizeof(mAdvPath), "%s/advertising", rootPath.get()); + g_strlcpy(mAdvUUID, aAdvUUID, sizeof(mAdvUUID)); if (aAdvName != nullptr) { - g_snprintf(mAdvName, sizeof(mAdvName), "%s", aAdvName); + g_strlcpy(mAdvName, aAdvName, sizeof(mAdvName)); } else { @@ -145,17 +145,17 @@ CHIP_ERROR BluezAdvertisement::Init(const BluezEndpoint & aEndpoint, const char CHIP_ERROR BluezAdvertisement::SetIntervals(AdvertisingIntervals aAdvIntervals) { - VerifyOrReturnError(mpAdv != nullptr, CHIP_ERROR_UNINITIALIZED); + VerifyOrReturnError(mAdv, CHIP_ERROR_UNINITIALIZED); // If the advertisement is already running, BlueZ will update the intervals // automatically. There is no need to stop and restart the advertisement. - bluez_leadvertisement1_set_min_interval(mpAdv, aAdvIntervals.first * 0.625); - bluez_leadvertisement1_set_max_interval(mpAdv, aAdvIntervals.second * 0.625); + bluez_leadvertisement1_set_min_interval(mAdv.get(), aAdvIntervals.first * 0.625); + bluez_leadvertisement1_set_max_interval(mAdv.get(), aAdvIntervals.second * 0.625); return CHIP_NO_ERROR; } CHIP_ERROR BluezAdvertisement::SetupServiceData(ServiceDataFlags aFlags) { - VerifyOrReturnError(mpAdv != nullptr, CHIP_ERROR_UNINITIALIZED); + VerifyOrReturnError(mAdv, CHIP_ERROR_UNINITIALIZED); Ble::ChipBLEDeviceIdentificationInfo deviceInfo; ReturnErrorOnFailure(ConfigurationMgr().GetBLEDeviceIdentificationInfo(deviceInfo)); @@ -177,7 +177,7 @@ CHIP_ERROR BluezAdvertisement::SetupServiceData(ServiceDataFlags aFlags) GVariantBuilder serviceDataBuilder; g_variant_builder_init(&serviceDataBuilder, G_VARIANT_TYPE("a{sv}")); - g_variant_builder_add(&serviceDataBuilder, "{sv}", mpAdvUUID, + g_variant_builder_add(&serviceDataBuilder, "{sv}", mAdvUUID, g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, &deviceInfo, sizeof(deviceInfo), sizeof(uint8_t))); GVariant * serviceData = g_variant_builder_end(&serviceDataBuilder); @@ -185,7 +185,7 @@ CHIP_ERROR BluezAdvertisement::SetupServiceData(ServiceDataFlags aFlags) GAutoPtr debugStr(g_variant_print(serviceData, TRUE)); ChipLogDetail(DeviceLayer, "SET service data to %s", StringOrNullMarker(debugStr.get())); - bluez_leadvertisement1_set_service_data(mpAdv, serviceData); + bluez_leadvertisement1_set_service_data(mAdv.get(), serviceData); return CHIP_NO_ERROR; } @@ -208,30 +208,17 @@ void BluezAdvertisement::Shutdown() // attached to the advertising object that may run on the glib thread. PlatformMgrImpl().GLibMatterContextInvokeSync( +[](BluezAdvertisement * self) { - if (self->mpRoot != nullptr) - { - g_object_unref(self->mpRoot); - self->mpRoot = nullptr; - } - if (self->mpAdapter != nullptr) - { - g_object_unref(self->mpAdapter); - self->mpAdapter = nullptr; - } - if (self->mpAdv != nullptr) - { - g_object_unref(self->mpAdv); - self->mpAdv = nullptr; - } + // The object manager server (mRoot) might not be released right away (it may be held + // by other BLE layer objects). We need to unexport the advertisement object in the + // explicit way to make sure that we can export it again in the Init() method. + g_dbus_object_manager_server_unexport(self->mRoot.get(), self->mAdvPath); + self->mRoot.reset(); + self->mAdapter.reset(); + self->mAdv.reset(); return CHIP_NO_ERROR; }, this); - g_free(mpAdvPath); - mpAdvPath = nullptr; - g_free(mpAdvUUID); - mpAdvUUID = nullptr; - mIsInitialized = false; } @@ -242,10 +229,6 @@ void BluezAdvertisement::StartDone(GObject * aObject, GAsyncResult * aResult) gboolean success = FALSE; success = bluez_leadvertising_manager1_call_register_advertisement_finish(advMgr, aResult, &error.GetReceiver()); - if (success == FALSE) - { - g_dbus_object_manager_server_unexport(mpRoot, mpAdvPath); - } VerifyOrExit(success == TRUE, ChipLogError(DeviceLayer, "FAIL: RegisterAdvertisement : %s", error->message)); mIsAdvertising = true; @@ -253,30 +236,30 @@ void BluezAdvertisement::StartDone(GObject * aObject, GAsyncResult * aResult) ChipLogDetail(DeviceLayer, "RegisterAdvertisement complete"); exit: - BLEManagerImpl::NotifyBLEPeripheralAdvStartComplete(success == TRUE, nullptr); + BLEManagerImpl::NotifyBLEPeripheralAdvStartComplete(success == TRUE); } CHIP_ERROR BluezAdvertisement::StartImpl() { - GDBusObject * adapter; - BluezLEAdvertisingManager1 * advMgr = nullptr; + GDBusObject * adapterObject; + GAutoPtr advMgr; GVariantBuilder optionsBuilder; GVariant * options; VerifyOrExit(!mIsAdvertising, ChipLogError(DeviceLayer, "FAIL: Advertising has already been enabled in %s", __func__)); - VerifyOrExit(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__)); - adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mpAdapter)); - VerifyOrExit(adapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapter in %s", __func__)); + adapterObject = g_dbus_interface_get_object(G_DBUS_INTERFACE(mAdapter.get())); + VerifyOrExit(adapterObject != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapterObject in %s", __func__)); - advMgr = bluez_object_get_leadvertising_manager1(BLUEZ_OBJECT(adapter)); - VerifyOrExit(advMgr != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL advMgr in %s", __func__)); + advMgr.reset(bluez_object_get_leadvertising_manager1(BLUEZ_OBJECT(adapterObject))); + VerifyOrExit(advMgr.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL advMgr in %s", __func__)); g_variant_builder_init(&optionsBuilder, G_VARIANT_TYPE("a{sv}")); options = g_variant_builder_end(&optionsBuilder); bluez_leadvertising_manager1_call_register_advertisement( - advMgr, mpAdvPath, options, nullptr, + advMgr.get(), mAdvPath, options, nullptr, [](GObject * aObject, GAsyncResult * aResult, void * aData) { reinterpret_cast(aData)->StartDone(aObject, aResult); }, @@ -304,40 +287,32 @@ void BluezAdvertisement::StopDone(GObject * aObject, GAsyncResult * aResult) gboolean success = FALSE; success = bluez_leadvertising_manager1_call_unregister_advertisement_finish(advMgr, aResult, &error.GetReceiver()); + VerifyOrExit(success == TRUE, ChipLogError(DeviceLayer, "FAIL: UnregisterAdvertisement: %s", error->message)); - if (success == FALSE) - { - g_dbus_object_manager_server_unexport(mpRoot, mpAdvPath); - } - else - { - mIsAdvertising = false; - } - - VerifyOrExit(success == TRUE, ChipLogError(DeviceLayer, "FAIL: UnregisterAdvertisement : %s", error->message)); + mIsAdvertising = false; ChipLogDetail(DeviceLayer, "UnregisterAdvertisement complete"); exit: - BLEManagerImpl::NotifyBLEPeripheralAdvStopComplete(success == TRUE, nullptr); + BLEManagerImpl::NotifyBLEPeripheralAdvStopComplete(success == TRUE); } CHIP_ERROR BluezAdvertisement::StopImpl() { - GDBusObject * adapter; - BluezLEAdvertisingManager1 * advMgr = nullptr; + GDBusObject * adapterObject; + GAutoPtr advMgr; VerifyOrExit(mIsAdvertising, ChipLogError(DeviceLayer, "FAIL: Advertising has already been disabled in %s", __func__)); - VerifyOrExit(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__)); - adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mpAdapter)); - VerifyOrExit(adapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapter in %s", __func__)); + adapterObject = g_dbus_interface_get_object(G_DBUS_INTERFACE(mAdapter.get())); + VerifyOrExit(adapterObject != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapterObject in %s", __func__)); - advMgr = bluez_object_get_leadvertising_manager1(BLUEZ_OBJECT(adapter)); - VerifyOrExit(advMgr != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL advMgr in %s", __func__)); + advMgr.reset(bluez_object_get_leadvertising_manager1(BLUEZ_OBJECT(adapterObject))); + VerifyOrExit(advMgr.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL advMgr in %s", __func__)); bluez_leadvertising_manager1_call_unregister_advertisement( - advMgr, mpAdvPath, nullptr, + advMgr.get(), mAdvPath, nullptr, [](GObject * aObject, GAsyncResult * aResult, void * aData) { reinterpret_cast(aData)->StopDone(aObject, aResult); }, diff --git a/src/platform/Linux/bluez/BluezAdvertisement.h b/src/platform/Linux/bluez/BluezAdvertisement.h index 11fe51c8e7cdb9..f41d591410b8e3 100644 --- a/src/platform/Linux/bluez/BluezAdvertisement.h +++ b/src/platform/Linux/bluez/BluezAdvertisement.h @@ -26,6 +26,7 @@ #include #include +#include #include #include "Types.h" @@ -64,6 +65,9 @@ class BluezAdvertisement /// /// BLE advertising is stopped asynchronously. Application will be notified of /// completion via a call to BLEManagerImpl::NotifyBLEPeripheralAdvStopComplete(). + /// + /// It is also possible that the advertising is released by BlueZ. In that case, + /// the application will be notified by BLEManagerImpl::NotifyBLEPeripheralAdvReleased(). CHIP_ERROR Stop(); private: @@ -79,15 +83,15 @@ class BluezAdvertisement CHIP_ERROR StopImpl(); // Objects (interfaces) used by LE advertisement - GDBusObjectManagerServer * mpRoot = nullptr; - BluezAdapter1 * mpAdapter = nullptr; - BluezLEAdvertisement1 * mpAdv = nullptr; + GAutoPtr mRoot; + GAutoPtr mAdapter; + GAutoPtr mAdv; bool mIsInitialized = false; bool mIsAdvertising = false; - char * mpAdvPath = nullptr; - char * mpAdvUUID = nullptr; + char mAdvPath[64] = ""; // D-Bus path of the advertisement object + char mAdvUUID[64] = ""; // UUID of the service to be advertised char mAdvName[32] = ""; }; diff --git a/src/platform/Linux/bluez/BluezConnection.cpp b/src/platform/Linux/bluez/BluezConnection.cpp index d2d5745a4c37c2..d2db5edc005eba 100644 --- a/src/platform/Linux/bluez/BluezConnection.cpp +++ b/src/platform/Linux/bluez/BluezConnection.cpp @@ -60,7 +60,7 @@ gboolean BluezIsCharOnService(BluezGattCharacteristic1 * aChar, BluezGattService } // namespace BluezConnection::BluezConnection(const BluezEndpoint & aEndpoint, BluezDevice1 * apDevice) : - mpDevice(BLUEZ_DEVICE1(g_object_ref(apDevice))) + mpDevice(reinterpret_cast(g_object_ref(apDevice))) { Init(aEndpoint); } @@ -101,9 +101,9 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) if (!aEndpoint.mIsCentral) { - mpService = BLUEZ_GATT_SERVICE1(g_object_ref(aEndpoint.mpService)); - mpC1 = BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC1)); - mpC2 = BLUEZ_GATT_CHARACTERISTIC1(g_object_ref(aEndpoint.mpC2)); + mpService = reinterpret_cast(g_object_ref(aEndpoint.mpService)); + mpC1 = reinterpret_cast(g_object_ref(aEndpoint.mpC1)); + mpC2 = reinterpret_cast(g_object_ref(aEndpoint.mpC2)); } else { @@ -111,9 +111,7 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) for (l = objects; l != nullptr; l = l->next) { - BluezObject * object = BLUEZ_OBJECT(l->data); - BluezGattService1 * service = bluez_object_get_gatt_service1(object); - + BluezGattService1 * service = bluez_object_get_gatt_service1(BLUEZ_OBJECT(l->data)); if (service != nullptr) { if ((BluezIsServiceOnDevice(service, mpDevice)) == TRUE && @@ -130,9 +128,7 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint) for (l = objects; l != nullptr; l = l->next) { - BluezObject * object = BLUEZ_OBJECT(l->data); - BluezGattCharacteristic1 * char1 = bluez_object_get_gatt_characteristic1(object); - + BluezGattCharacteristic1 * char1 = bluez_object_get_gatt_characteristic1(BLUEZ_OBJECT(l->data)); if (char1 != nullptr) { if ((BluezIsCharOnService(char1, mpService) == TRUE) && diff --git a/src/platform/Linux/bluez/BluezEndpoint.cpp b/src/platform/Linux/bluez/BluezEndpoint.cpp index e68b28b8f421a8..2fdd1e1788af6b 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.cpp +++ b/src/platform/Linux/bluez/BluezEndpoint.cpp @@ -268,33 +268,33 @@ void BluezEndpoint::RegisterGattApplicationDone(GObject * aObject, GAsyncResult VerifyOrReturn(success == TRUE, { ChipLogError(DeviceLayer, "FAIL: RegisterApplication : %s", error->message); - BLEManagerImpl::NotifyBLEPeripheralRegisterAppComplete(false, nullptr); + BLEManagerImpl::NotifyBLEPeripheralRegisterAppComplete(false); }); - BLEManagerImpl::NotifyBLEPeripheralRegisterAppComplete(true, nullptr); + BLEManagerImpl::NotifyBLEPeripheralRegisterAppComplete(true); ChipLogDetail(DeviceLayer, "BluezPeripheralRegisterAppDone done"); } CHIP_ERROR BluezEndpoint::RegisterGattApplicationImpl() { - GDBusObject * adapter; - BluezGattManager1 * gattMgr; + GDBusObject * adapterObject; + GAutoPtr gattMgr; GVariantBuilder optionsBuilder; GVariant * options; - VerifyOrExit(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__)); - adapter = g_dbus_interface_get_object(G_DBUS_INTERFACE(mpAdapter)); - VerifyOrExit(adapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapter in %s", __func__)); + adapterObject = g_dbus_interface_get_object(G_DBUS_INTERFACE(mAdapter.get())); + VerifyOrExit(adapterObject != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL adapterObject in %s", __func__)); - gattMgr = bluez_object_get_gatt_manager1(BLUEZ_OBJECT(adapter)); - VerifyOrExit(gattMgr != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL gattMgr in %s", __func__)); + gattMgr.reset(bluez_object_get_gatt_manager1(BLUEZ_OBJECT(adapterObject))); + VerifyOrExit(gattMgr.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL gattMgr in %s", __func__)); g_variant_builder_init(&optionsBuilder, G_VARIANT_TYPE("a{sv}")); options = g_variant_builder_end(&optionsBuilder); bluez_gatt_manager1_call_register_application( - gattMgr, mpRootPath, options, nullptr, + gattMgr.get(), mpRootPath, options, nullptr, +[](GObject * aObj, GAsyncResult * aResult, void * self) { reinterpret_cast(self)->RegisterGattApplicationDone(aObj, aResult); }, @@ -344,11 +344,11 @@ void BluezEndpoint::BluezSignalInterfacePropertiesChanged(GDBusObjectManagerClie GDBusProxy * aInterface, GVariant * aChangedProperties, const char * const * aInvalidatedProps) { - VerifyOrReturn(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrReturn(mAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__)); VerifyOrReturn(strcmp(g_dbus_proxy_get_interface_name(aInterface), DEVICE_INTERFACE) == 0, ); BluezDevice1 * device = BLUEZ_DEVICE1(aInterface); - VerifyOrReturn(BluezIsDeviceOnAdapter(device, mpAdapter)); + VerifyOrReturn(BluezIsDeviceOnAdapter(device, mAdapter.get())); UpdateConnectionTable(device); } @@ -386,7 +386,7 @@ void BluezEndpoint::BluezSignalOnObjectAdded(GDBusObjectManager * aManager, GDBu GAutoPtr device(bluez_object_get_device1(BLUEZ_OBJECT(aObject))); VerifyOrReturn(device.get() != nullptr); - if (BluezIsDeviceOnAdapter(device.get(), mpAdapter) == TRUE) + if (BluezIsDeviceOnAdapter(device.get(), mAdapter.get()) == TRUE) { HandleNewDevice(device.get()); } @@ -395,7 +395,7 @@ void BluezEndpoint::BluezSignalOnObjectAdded(GDBusObjectManager * aManager, GDBu void BluezEndpoint::BluezSignalOnObjectRemoved(GDBusObjectManager * aManager, GDBusObject * aObject) { // TODO: for Device1, lookup connection, and call otPlatTobleHandleDisconnected - // for Adapter1: unclear, crash if this pertains to our adapter? at least null out the self->mpAdapter. + // for Adapter1: unclear, crash if this pertains to our adapter? at least null out the self->mAdapter. // for Characteristic1, or GattService -- handle here via calling otPlatTobleHandleDisconnected, or ignore. } @@ -427,43 +427,38 @@ void BluezEndpoint::SetupAdapter() snprintf(expectedPath, sizeof(expectedPath), BLUEZ_PATH "/hci%u", mAdapterId); GList * objects = g_dbus_object_manager_get_objects(mpObjMgr); - for (auto l = objects; l != nullptr && mpAdapter == nullptr; l = l->next) + for (auto l = objects; l != nullptr && mAdapter.get() == nullptr; l = l->next) { - BluezObject * object = BLUEZ_OBJECT(l->data); - - GList * interfaces = g_dbus_object_get_interfaces(G_DBUS_OBJECT(object)); - for (auto ll = interfaces; ll != nullptr; ll = ll->next) + GAutoPtr adapter(bluez_object_get_adapter1(BLUEZ_OBJECT(l->data))); + if (adapter.get() != nullptr) { - if (BLUEZ_IS_ADAPTER1(ll->data)) - { // we found the adapter - BluezAdapter1 * adapter = BLUEZ_ADAPTER1(ll->data); - if (mpAdapterAddr == nullptr) // no adapter address provided, bind to the hci indicated by nodeid + if (mpAdapterAddr == nullptr) // no adapter address provided, bind to the hci indicated by nodeid + { + if (strcmp(g_dbus_proxy_get_object_path(G_DBUS_PROXY(adapter.get())), expectedPath) == 0) { - if (strcmp(g_dbus_proxy_get_object_path(G_DBUS_PROXY(adapter)), expectedPath) == 0) - { - mpAdapter = static_cast(g_object_ref(adapter)); - } + mAdapter.reset(static_cast(g_object_ref(adapter.get()))); + break; } - else + } + else + { + if (strcmp(bluez_adapter1_get_address(adapter.get()), mpAdapterAddr) == 0) { - if (strcmp(bluez_adapter1_get_address(adapter), mpAdapterAddr) == 0) - { - mpAdapter = static_cast(g_object_ref(adapter)); - } + mAdapter.reset(static_cast(g_object_ref(adapter.get()))); + break; } } } - g_list_free_full(interfaces, g_object_unref); } - VerifyOrExit(mpAdapter != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mpAdapter in %s", __func__)); + VerifyOrExit(mAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__)); - bluez_adapter1_set_powered(mpAdapter, TRUE); + bluez_adapter1_set_powered(mAdapter.get(), TRUE); // Setting "Discoverable" to False on the adapter and to True on the advertisement convinces // Bluez to set "BR/EDR Not Supported" flag. Bluez doesn't provide API to do that explicitly // and the flag is necessary to force using LE transport. - bluez_adapter1_set_discoverable(mpAdapter, FALSE); + bluez_adapter1_set_discoverable(mAdapter.get(), FALSE); exit: g_list_free_full(objects, g_object_unref); @@ -692,8 +687,7 @@ void BluezEndpoint::Shutdown() +[](BluezEndpoint * self) { if (self->mpObjMgr != nullptr) g_object_unref(self->mpObjMgr); - if (self->mpAdapter != nullptr) - g_object_unref(self->mpAdapter); + self->mAdapter.reset(); if (self->mpRoot != nullptr) g_object_unref(self->mpRoot); if (self->mpService != nullptr) diff --git a/src/platform/Linux/bluez/BluezEndpoint.h b/src/platform/Linux/bluez/BluezEndpoint.h index 115731d665d4ba..d35cebdf6a6cfe 100644 --- a/src/platform/Linux/bluez/BluezEndpoint.h +++ b/src/platform/Linux/bluez/BluezEndpoint.h @@ -74,7 +74,7 @@ class BluezEndpoint CHIP_ERROR Init(bool aIsCentral, const char * apBleAddr); void Shutdown(); - BluezAdapter1 * GetAdapter() const { return mpAdapter; } + BluezAdapter1 * GetAdapter() const { return mAdapter.get(); } CHIP_ERROR RegisterGattApplication(); GDBusObjectManagerServer * GetGattApplicationObjectManager() const { return mpRoot; } @@ -127,7 +127,7 @@ class BluezEndpoint // Objects (interfaces) subscribed to by this service GDBusObjectManager * mpObjMgr = nullptr; - BluezAdapter1 * mpAdapter = nullptr; + GAutoPtr mAdapter; // Objects (interfaces) published by this service GDBusObjectManagerServer * mpRoot = nullptr; diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 03b26aa08e3dc7..0d248058c3d1a0 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -58,7 +58,7 @@ CHIP_ERROR ChipDeviceScanner::Init(BluezAdapter1 * adapter, ChipDeviceScannerDel // Make this function idempotent by shutting down previously initialized state if any. Shutdown(); - mAdapter = BLUEZ_ADAPTER1(g_object_ref(adapter)); + mAdapter.reset(reinterpret_cast(g_object_ref(adapter))); mDelegate = delegate; // Create the D-Bus object manager client object on the glib thread, so that all D-Bus signals @@ -97,8 +97,7 @@ void ChipDeviceScanner::Shutdown() +[](ChipDeviceScanner * self) { if (self->mManager != nullptr) g_object_unref(self->mManager); - if (self->mAdapter != nullptr) - g_object_unref(self->mAdapter); + self->mAdapter.reset(); return CHIP_NO_ERROR; }, this); @@ -205,7 +204,7 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStopScan(ChipDeviceScanner * self) self->mInterfaceChangedSignal = 0; } - if (!bluez_adapter1_call_stop_discovery_sync(self->mAdapter, nullptr /* not cancellable */, &error.GetReceiver())) + if (!bluez_adapter1_call_stop_discovery_sync(self->mAdapter.get(), nullptr /* not cancellable */, &error.GetReceiver())) { ChipLogError(Ble, "Failed to stop discovery %s", error->message); return CHIP_ERROR_INTERNAL; @@ -234,7 +233,7 @@ void ChipDeviceScanner::SignalInterfaceChanged(GDBusObjectManagerClient * manage void ChipDeviceScanner::ReportDevice(BluezDevice1 & device) { - if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mAdapter))) != 0) + if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mAdapter.get()))) != 0) { return; } @@ -252,7 +251,7 @@ void ChipDeviceScanner::ReportDevice(BluezDevice1 & device) void ChipDeviceScanner::RemoveDevice(BluezDevice1 & device) { - if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mAdapter))) != 0) + if (strcmp(bluez_device1_get_adapter(&device), g_dbus_proxy_get_object_path(G_DBUS_PROXY(mAdapter.get()))) != 0) { return; } @@ -267,7 +266,7 @@ void ChipDeviceScanner::RemoveDevice(BluezDevice1 & device) const auto devicePath = g_dbus_proxy_get_object_path(G_DBUS_PROXY(&device)); GAutoPtr error; - if (!bluez_adapter1_call_remove_device_sync(mAdapter, devicePath, nullptr, &error.GetReceiver())) + if (!bluez_adapter1_call_remove_device_sync(mAdapter.get(), devicePath, nullptr, &error.GetReceiver())) { ChipLogDetail(Ble, "Failed to remove device %s: %s", StringOrNullMarker(devicePath), error->message); } @@ -302,7 +301,8 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) g_variant_builder_add(&filterBuilder, "{sv}", "Transport", g_variant_new_string("le")); GVariant * filter = g_variant_builder_end(&filterBuilder); - if (!bluez_adapter1_call_set_discovery_filter_sync(self->mAdapter, filter, self->mCancellable.get(), &error.GetReceiver())) + if (!bluez_adapter1_call_set_discovery_filter_sync(self->mAdapter.get(), filter, self->mCancellable.get(), + &error.GetReceiver())) { // Not critical: ignore if fails ChipLogError(Ble, "Failed to set discovery filters: %s", error->message); @@ -310,7 +310,7 @@ CHIP_ERROR ChipDeviceScanner::MainLoopStartScan(ChipDeviceScanner * self) } ChipLogProgress(Ble, "BLE initiating scan."); - if (!bluez_adapter1_call_start_discovery_sync(self->mAdapter, self->mCancellable.get(), &error.GetReceiver())) + if (!bluez_adapter1_call_start_discovery_sync(self->mAdapter.get(), self->mCancellable.get(), &error.GetReceiver())) { ChipLogError(Ble, "Failed to start discovery: %s", error->message); return CHIP_ERROR_INTERNAL; diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.h b/src/platform/Linux/bluez/ChipDeviceScanner.h index 1271ec39cb241a..276499d73ef82a 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.h +++ b/src/platform/Linux/bluez/ChipDeviceScanner.h @@ -27,6 +27,8 @@ #include #include +#include "Types.h" + namespace chip { namespace DeviceLayer { namespace Internal { @@ -105,8 +107,8 @@ class ChipDeviceScanner /// so that it can be re-discovered if it's still advertising. void RemoveDevice(BluezDevice1 & device); - GDBusObjectManager * mManager = nullptr; - BluezAdapter1 * mAdapter = nullptr; + GDBusObjectManager * mManager = nullptr; + GAutoPtr mAdapter; ChipDeviceScannerDelegate * mDelegate = nullptr; gulong mObjectAddedSignal = 0; gulong mInterfaceChangedSignal = 0; diff --git a/src/platform/Linux/bluez/Types.h b/src/platform/Linux/bluez/Types.h index f7be4d858d079a..729f3b445224af 100644 --- a/src/platform/Linux/bluez/Types.h +++ b/src/platform/Linux/bluez/Types.h @@ -54,12 +54,36 @@ namespace chip { +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + template <> struct GAutoPtrDeleter { using deleter = GObjectDeleter; }; +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + +template <> +struct GAutoPtrDeleter +{ + using deleter = GObjectDeleter; +}; + namespace DeviceLayer { namespace Internal { From ef8ee3239de27f1ca9290b51738ae407922d4c16 Mon Sep 17 00:00:00 2001 From: Vivien Nicolas Date: Fri, 23 Feb 2024 11:19:12 +0100 Subject: [PATCH 2/3] [DiagnosticLogs] Allow TransferFileDesignator or size 0 (#32267) --- .../common/BDXDiagnosticLogsServerDelegate.cpp | 13 ++++++++++--- .../diagnostic-logs-server.cpp | 2 -- src/protocols/bdx/BdxTransferProxyDiagnosticLog.cpp | 1 - src/protocols/bdx/StatusCode.cpp | 8 ++++++++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/examples/chip-tool/commands/common/BDXDiagnosticLogsServerDelegate.cpp b/examples/chip-tool/commands/common/BDXDiagnosticLogsServerDelegate.cpp index 27260a54f57887..2a5b2d5d7c97a1 100644 --- a/examples/chip-tool/commands/common/BDXDiagnosticLogsServerDelegate.cpp +++ b/examples/chip-tool/commands/common/BDXDiagnosticLogsServerDelegate.cpp @@ -25,9 +25,10 @@ constexpr uint8_t kMaxFileDesignatorLen = 32; constexpr uint16_t kMaxFilePathLen = kMaxFileDesignatorLen + sizeof(kTmpDir) + 1; // For testing a few file names trigger an error depending on the current 'phase'. -constexpr char kErrorOnTransferBegin[] = "Error:OnTransferBegin"; -constexpr char kErrorOnTransferData[] = "Error:OnTransferData"; -constexpr char kErrorOnTransferEnd[] = "Error:OnTransferEnd"; +constexpr char kErrorOnTransferBegin[] = "Error:OnTransferBegin"; +constexpr char kErrorOnTransferData[] = "Error:OnTransferData"; +constexpr char kErrorOnTransferEnd[] = "Error:OnTransferEnd"; +constexpr char kErrorTransferMethodNotSupported[] = "TransferMethodNotSupported.txt"; BDXDiagnosticLogsServerDelegate BDXDiagnosticLogsServerDelegate::sInstance; @@ -136,8 +137,14 @@ CHIP_ERROR BDXDiagnosticLogsServerDelegate::OnTransferBegin(chip::bdx::BDXTransf auto fileDesignator = transfer->GetFileDesignator(); LogFileDesignator("OnTransferBegin", fileDesignator); + VerifyOrReturnError(fileDesignator.size() != 0, CHIP_ERROR_UNKNOWN_RESOURCE_ID); + chip::CharSpan phaseErrorTarget(kErrorOnTransferBegin, sizeof(kErrorOnTransferBegin) - 1); ReturnErrorOnFailure(CheckForErrorRequested(phaseErrorTarget, fileDesignator)); + + chip::CharSpan transferErrorTarget(kErrorTransferMethodNotSupported, sizeof(kErrorTransferMethodNotSupported) - 1); + VerifyOrReturnError(!transferErrorTarget.data_equal(fileDesignator), CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE); + ReturnErrorOnFailure(CheckFileDesignatorAllowed(mFileDesignators, fileDesignator)); char outputFilePath[kMaxFilePathLen] = { 0 }; diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp index 7b1b036ee4857b..83beb46484bb71 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp @@ -132,8 +132,6 @@ void DiagnosticLogsServer::HandleLogRequestForBdx(CommandHandler * commandObj, c // INVALID_COMMAND. VerifyOrReturn(transferFileDesignator.HasValue(), commandObj->AddStatus(path, Status::InvalidCommand)); - VerifyOrReturn(transferFileDesignator.Value().size() > 0, commandObj->AddStatus(path, Status::ConstraintError)); - VerifyOrReturn(transferFileDesignator.Value().size() <= kMaxFileDesignatorLen, commandObj->AddStatus(path, Status::ConstraintError)); diff --git a/src/protocols/bdx/BdxTransferProxyDiagnosticLog.cpp b/src/protocols/bdx/BdxTransferProxyDiagnosticLog.cpp index 3f8915462941fb..ba1a41376afbbe 100644 --- a/src/protocols/bdx/BdxTransferProxyDiagnosticLog.cpp +++ b/src/protocols/bdx/BdxTransferProxyDiagnosticLog.cpp @@ -33,7 +33,6 @@ CHIP_ERROR BDXTransferProxyDiagnosticLog::Init(TransferSession * transferSession uint16_t fileDesignatorLength = 0; auto fileDesignator = transferSession->GetFileDesignator(fileDesignatorLength); - VerifyOrReturnError(fileDesignatorLength > 0, CHIP_ERROR_INVALID_STRING_LENGTH); VerifyOrReturnError(fileDesignatorLength <= ArraySize(mFileDesignator), CHIP_ERROR_INVALID_STRING_LENGTH); mTransfer = transferSession; diff --git a/src/protocols/bdx/StatusCode.cpp b/src/protocols/bdx/StatusCode.cpp index 2f739b5e831de2..9ab6f8d9b442be 100644 --- a/src/protocols/bdx/StatusCode.cpp +++ b/src/protocols/bdx/StatusCode.cpp @@ -33,6 +33,14 @@ StatusCode GetBdxStatusCodeFromChipError(CHIP_ERROR error) { status = StatusCode::kBadMessageContents; } + else if (error == CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE) + { + status = StatusCode::kTransferMethodNotSupported; + } + else if (error == CHIP_ERROR_UNKNOWN_RESOURCE_ID) + { + status = StatusCode::kFileDesignatorUnknown; + } return status; } From a6fd6ce388bcb120b3d0128779fa0ecf40a5ee16 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 23 Feb 2024 09:55:29 -0500 Subject: [PATCH 3/3] Create a separate 'constants' source set in `src/app` (#32228) * Create constants file * Restyle * Place everything into a `Revision` namespace to avoid name collision with a Tags enum * Rename --------- Co-authored-by: Andrei Litvin --- src/app/BUILD.gn | 23 +++++++------ src/app/DataModelRevision.h | 31 ----------------- src/app/InteractionModelTimeout.h | 1 - src/app/MessageDef/BUILD.gn | 2 +- src/app/MessageDef/InvokeRequestMessage.cpp | 2 +- src/app/MessageDef/InvokeResponseMessage.cpp | 2 +- src/app/MessageDef/MessageBuilder.cpp | 3 +- src/app/MessageDef/MessageBuilder.h | 2 +- src/app/MessageDef/MessageDefHelper.cpp | 2 +- src/app/MessageDef/MessageParser.cpp | 4 +-- src/app/MessageDef/MessageParser.h | 2 +- src/app/MessageDef/ReadRequestMessage.cpp | 2 +- src/app/MessageDef/ReportDataMessage.cpp | 2 +- src/app/MessageDef/StatusResponseMessage.cpp | 2 +- .../MessageDef/SubscribeRequestMessage.cpp | 2 +- .../MessageDef/SubscribeResponseMessage.cpp | 2 +- src/app/MessageDef/TimedRequestMessage.cpp | 2 +- src/app/MessageDef/WriteRequestMessage.cpp | 2 +- src/app/MessageDef/WriteResponseMessage.cpp | 2 +- ...sion.h => SpecificationDefinedRevisions.h} | 33 +++++++++++++++---- src/app/SpecificationVersion.h | 31 ----------------- .../basic-information/basic-information.cpp | 7 ++-- src/protocols/secure_channel/BUILD.gn | 2 +- .../secure_channel/PairingSession.cpp | 10 +++--- 24 files changed, 65 insertions(+), 108 deletions(-) delete mode 100644 src/app/DataModelRevision.h rename src/app/{InteractionModelRevision.h => SpecificationDefinedRevisions.h} (51%) delete mode 100644 src/app/SpecificationVersion.h diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index f714560afc15cb..69f5926909756d 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -76,14 +76,6 @@ source_set("app_config") { deps = [ ":app_buildconfig" ] } -source_set("revision_info") { - sources = [ - "DataModelRevision.h", - "InteractionModelRevision.h", - "SpecificationVersion.h", - ] -} - source_set("paths") { sources = [ "AttributePathParams.h", @@ -130,6 +122,17 @@ config("config-controller-dynamic-server") { ] } +source_set("constants") { + sources = [ + "InteractionModelTimeout.h", + "SpecificationDefinedRevisions.h", + ] + public_deps = [ + "${chip_root}/src/lib/core", + "${chip_root}/src/system", + ] +} + # interaction-model is a static-library because it currently requires global functions (app/util/...) that are stubbed in different test files that depend on the app static_library # which in tern depens on the interaction-model. # Using source_set prevents the unit test to build correctly. @@ -149,7 +152,6 @@ static_library("interaction-model") { "InteractionModelEngine.cpp", "InteractionModelEngine.h", "InteractionModelHelper.h", - "InteractionModelTimeout.h", "OperationalSessionSetup.cpp", "OperationalSessionSetup.h", "OperationalSessionSetupPool.h", @@ -181,6 +183,7 @@ static_library("interaction-model") { public_deps = [ ":app_config", + ":constants", ":paths", ":subscription-manager", "${chip_root}/src/app/MessageDef", @@ -276,9 +279,9 @@ static_library("app") { public_deps = [ ":app_config", + ":constants", ":global-attributes", ":interaction-model", - ":revision_info", "${chip_root}/src/app/data-model", "${chip_root}/src/app/icd/server:icd-server-config", "${chip_root}/src/lib/address_resolve", diff --git a/src/app/DataModelRevision.h b/src/app/DataModelRevision.h deleted file mode 100644 index 47ecb626a13ffa..00000000000000 --- a/src/app/DataModelRevision.h +++ /dev/null @@ -1,31 +0,0 @@ -/* - * - * Copyright (c) 2022 Project CHIP Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -/** - * CHIP_DEVICE_DATA_MODEL_REVISION - * - * A monotonic number identifying the revision number of the Data Model against - * which the Node is certified. - * - * See section 7.1.1. "Revision History" in the "Data Model Specification" - * chapter of the core Matter specification. - */ -#ifndef CHIP_DEVICE_DATA_MODEL_REVISION -#define CHIP_DEVICE_DATA_MODEL_REVISION 17 -#endif diff --git a/src/app/InteractionModelTimeout.h b/src/app/InteractionModelTimeout.h index 3511ed5412049d..43153c679f2361 100644 --- a/src/app/InteractionModelTimeout.h +++ b/src/app/InteractionModelTimeout.h @@ -18,7 +18,6 @@ #pragma once #include -#include namespace chip { namespace app { diff --git a/src/app/MessageDef/BUILD.gn b/src/app/MessageDef/BUILD.gn index 4880a9a8b126e4..18b254038fe2b8 100644 --- a/src/app/MessageDef/BUILD.gn +++ b/src/app/MessageDef/BUILD.gn @@ -113,8 +113,8 @@ source_set("MessageDef") { deps = [ "${chip_root}/src/app:app_config", + "${chip_root}/src/app:constants", "${chip_root}/src/app:paths", - "${chip_root}/src/app:revision_info", "${chip_root}/src/lib/core", "${chip_root}/src/lib/support", "${chip_root}/src/protocols/interaction_model", diff --git a/src/app/MessageDef/InvokeRequestMessage.cpp b/src/app/MessageDef/InvokeRequestMessage.cpp index 50bd5cd45f70c0..d334b05dfa0362 100644 --- a/src/app/MessageDef/InvokeRequestMessage.cpp +++ b/src/app/MessageDef/InvokeRequestMessage.cpp @@ -74,7 +74,7 @@ CHIP_ERROR InvokeRequestMessage::Parser::PrettyPrint() const PRETTY_PRINT_DECDEPTH(); } break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/MessageDef/InvokeResponseMessage.cpp b/src/app/MessageDef/InvokeResponseMessage.cpp index f9887530c15e15..53568ce14d9b5e 100644 --- a/src/app/MessageDef/InvokeResponseMessage.cpp +++ b/src/app/MessageDef/InvokeResponseMessage.cpp @@ -73,7 +73,7 @@ CHIP_ERROR InvokeResponseMessage::Parser::PrettyPrint() const } #endif // CHIP_DETAIL_LOGGING break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/MessageDef/MessageBuilder.cpp b/src/app/MessageDef/MessageBuilder.cpp index b9ad0e58968d23..4effd887c6814c 100644 --- a/src/app/MessageDef/MessageBuilder.cpp +++ b/src/app/MessageDef/MessageBuilder.cpp @@ -24,8 +24,7 @@ namespace chip { namespace app { CHIP_ERROR MessageBuilder::EncodeInteractionModelRevision() { - return mpWriter->Put(TLV::ContextTag(kInteractionModelRevisionTag), - static_cast(CHIP_DEVICE_INTERACTION_MODEL_REVISION)); + return mpWriter->Put(TLV::ContextTag(Revision::kInteractionModelRevisionTag), Revision::kInteractionModelRevision); } } // namespace app } // namespace chip diff --git a/src/app/MessageDef/MessageBuilder.h b/src/app/MessageDef/MessageBuilder.h index 03210b3011cf21..fac114ad1379b5 100644 --- a/src/app/MessageDef/MessageBuilder.h +++ b/src/app/MessageDef/MessageBuilder.h @@ -18,7 +18,7 @@ #pragma once #include "StructBuilder.h" -#include +#include #include namespace chip { diff --git a/src/app/MessageDef/MessageDefHelper.cpp b/src/app/MessageDef/MessageDefHelper.cpp index db24b2623dcc00..82fcf286d1751b 100644 --- a/src/app/MessageDef/MessageDefHelper.cpp +++ b/src/app/MessageDef/MessageDefHelper.cpp @@ -24,7 +24,7 @@ #include "MessageDefHelper.h" #include #include -#include +#include #include #include #include diff --git a/src/app/MessageDef/MessageParser.cpp b/src/app/MessageDef/MessageParser.cpp index b855a93afcd7eb..7d74378b49f0c2 100644 --- a/src/app/MessageDef/MessageParser.cpp +++ b/src/app/MessageDef/MessageParser.cpp @@ -16,7 +16,7 @@ #include "MessageParser.h" #include "MessageDefHelper.h" -#include +#include namespace chip { namespace app { @@ -52,7 +52,7 @@ CHIP_ERROR MessageParser::CheckInteractionModelRevision(TLV::TLVReader & aReader CHIP_ERROR MessageParser::GetInteractionModelRevision(InteractionModelRevision * const apInteractionModelRevision) const { - return GetUnsignedInteger(kInteractionModelRevisionTag, apInteractionModelRevision); + return GetUnsignedInteger(Revision::kInteractionModelRevisionTag, apInteractionModelRevision); } } // namespace app diff --git a/src/app/MessageDef/MessageParser.h b/src/app/MessageDef/MessageParser.h index e307d5b905d965..4886f295e253eb 100644 --- a/src/app/MessageDef/MessageParser.h +++ b/src/app/MessageDef/MessageParser.h @@ -20,7 +20,7 @@ #include "StructParser.h" #include -#include +#include #include namespace chip { diff --git a/src/app/MessageDef/ReadRequestMessage.cpp b/src/app/MessageDef/ReadRequestMessage.cpp index 8b96b044d774c5..855d0a3e14d936 100644 --- a/src/app/MessageDef/ReadRequestMessage.cpp +++ b/src/app/MessageDef/ReadRequestMessage.cpp @@ -91,7 +91,7 @@ CHIP_ERROR ReadRequestMessage::Parser::PrettyPrint() const } #endif // CHIP_DETAIL_LOGGING break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/MessageDef/ReportDataMessage.cpp b/src/app/MessageDef/ReportDataMessage.cpp index c3c569572ae9fe..a2a8ec47968ec1 100644 --- a/src/app/MessageDef/ReportDataMessage.cpp +++ b/src/app/MessageDef/ReportDataMessage.cpp @@ -112,7 +112,7 @@ CHIP_ERROR ReportDataMessage::Parser::PrettyPrint() const } #endif // CHIP_DETAIL_LOGGING break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/MessageDef/StatusResponseMessage.cpp b/src/app/MessageDef/StatusResponseMessage.cpp index f702998c90ed23..fbf3a870d542c6 100644 --- a/src/app/MessageDef/StatusResponseMessage.cpp +++ b/src/app/MessageDef/StatusResponseMessage.cpp @@ -52,7 +52,7 @@ CHIP_ERROR StatusResponseMessage::Parser::PrettyPrint() const } #endif // CHIP_DETAIL_LOGGING break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/MessageDef/SubscribeRequestMessage.cpp b/src/app/MessageDef/SubscribeRequestMessage.cpp index b536570891bade..7224c850baaa20 100644 --- a/src/app/MessageDef/SubscribeRequestMessage.cpp +++ b/src/app/MessageDef/SubscribeRequestMessage.cpp @@ -115,7 +115,7 @@ CHIP_ERROR SubscribeRequestMessage::Parser::PrettyPrint() const } #endif // CHIP_DETAIL_LOGGING break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/MessageDef/SubscribeResponseMessage.cpp b/src/app/MessageDef/SubscribeResponseMessage.cpp index 99528895e49796..25ef8ec0bd3038 100644 --- a/src/app/MessageDef/SubscribeResponseMessage.cpp +++ b/src/app/MessageDef/SubscribeResponseMessage.cpp @@ -59,7 +59,7 @@ CHIP_ERROR SubscribeResponseMessage::Parser::PrettyPrint() const } #endif // CHIP_DETAIL_LOGGING break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/MessageDef/TimedRequestMessage.cpp b/src/app/MessageDef/TimedRequestMessage.cpp index bf6bdc5b79a45b..830a3d66894377 100644 --- a/src/app/MessageDef/TimedRequestMessage.cpp +++ b/src/app/MessageDef/TimedRequestMessage.cpp @@ -49,7 +49,7 @@ CHIP_ERROR TimedRequestMessage::Parser::PrettyPrint() const } #endif // CHIP_DETAIL_LOGGING break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/MessageDef/WriteRequestMessage.cpp b/src/app/MessageDef/WriteRequestMessage.cpp index 8cf3d4048d9b2f..5278fff2c94166 100644 --- a/src/app/MessageDef/WriteRequestMessage.cpp +++ b/src/app/MessageDef/WriteRequestMessage.cpp @@ -88,7 +88,7 @@ CHIP_ERROR WriteRequestMessage::Parser::PrettyPrint() const } #endif // CHIP_DETAIL_LOGGING break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/MessageDef/WriteResponseMessage.cpp b/src/app/MessageDef/WriteResponseMessage.cpp index 4426d5e3db7c5e..e8da708bfc94e5 100644 --- a/src/app/MessageDef/WriteResponseMessage.cpp +++ b/src/app/MessageDef/WriteResponseMessage.cpp @@ -54,7 +54,7 @@ CHIP_ERROR WriteResponseMessage::Parser::PrettyPrint() const ReturnErrorOnFailure(writeResponses.PrettyPrint()); PRETTY_PRINT_DECDEPTH(); break; - case kInteractionModelRevisionTag: + case Revision::kInteractionModelRevisionTag: ReturnErrorOnFailure(MessageParser::CheckInteractionModelRevision(reader)); break; default: diff --git a/src/app/InteractionModelRevision.h b/src/app/SpecificationDefinedRevisions.h similarity index 51% rename from src/app/InteractionModelRevision.h rename to src/app/SpecificationDefinedRevisions.h index 529ac0f839c80e..cad67aced413b0 100644 --- a/src/app/InteractionModelRevision.h +++ b/src/app/SpecificationDefinedRevisions.h @@ -20,16 +20,37 @@ #include #include +#include + +namespace chip { +namespace Revision { + /** - * CHIP_DEVICE_INTERACTION_MODEL_REVISION - * * A monothonic number identifying the interaction model revision. * * See section 8.1.1. "Revision History" in the "Interaction Model * Specification" chapter of the core Matter specification. */ -#ifndef CHIP_DEVICE_INTERACTION_MODEL_REVISION -#define CHIP_DEVICE_INTERACTION_MODEL_REVISION 11 -#endif +inline constexpr InteractionModelRevision kInteractionModelRevision = 11; +inline constexpr uint8_t kInteractionModelRevisionTag = 0xFF; + +/** + * A monotonic number identifying the revision number of the Data Model against + * which the Node is certified. + * + * See section 7.1.1. "Revision History" in the "Data Model Specification" + * chapter of the core Matter specification. + */ +inline constexpr uint16_t kDataModelRevision = 17; + +/* + * A number identifying the specification version against which the + * Node is certified. + * + * See section 11.1.5.22. "SpecificationVersion Attribute" in "Service and + * Device Management" chapter of the core Matter specification. + */ +inline constexpr uint32_t kSpecificationVersion = 0x01030000; -inline constexpr uint8_t kInteractionModelRevisionTag = 0xFF; +} // namespace Revision +} // namespace chip diff --git a/src/app/SpecificationVersion.h b/src/app/SpecificationVersion.h deleted file mode 100644 index b0570dad14c4ce..00000000000000 --- a/src/app/SpecificationVersion.h +++ /dev/null @@ -1,31 +0,0 @@ -/* - * - * Copyright (c) 2023 Project CHIP Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -/** - * CHIP_DEVICE_SPECIFICATION_VERSION - * - * A number identifying the specification version against which the - * Node is certified. - * - * See section 11.1.5.22. "SpecificationVersion Attribute" in "Service and - * Device Management" chapter of the core Matter specification. - */ -#ifndef CHIP_DEVICE_SPECIFICATION_VERSION -#define CHIP_DEVICE_SPECIFICATION_VERSION 0x01030000 -#endif diff --git a/src/app/clusters/basic-information/basic-information.cpp b/src/app/clusters/basic-information/basic-information.cpp index 641dc59a853dd6..91eef9d9dae899 100644 --- a/src/app/clusters/basic-information/basic-information.cpp +++ b/src/app/clusters/basic-information/basic-information.cpp @@ -20,10 +20,9 @@ #include #include -#include #include #include -#include +#include #include #include #include @@ -314,7 +313,7 @@ CHIP_ERROR BasicAttrAccess::Read(const ConcreteReadAttributePath & aPath, Attrib CHIP_ERROR BasicAttrAccess::ReadDataModelRevision(AttributeValueEncoder & aEncoder) { - uint16_t revision = CHIP_DEVICE_DATA_MODEL_REVISION; + uint16_t revision = Revision::kDataModelRevision; return aEncoder.Encode(revision); } @@ -399,7 +398,7 @@ CHIP_ERROR BasicAttrAccess::ReadProductAppearance(AttributeValueEncoder & aEncod CHIP_ERROR BasicAttrAccess::ReadSpecificationVersion(AttributeValueEncoder & aEncoder) { - uint32_t specification_version = CHIP_DEVICE_SPECIFICATION_VERSION; + uint32_t specification_version = Revision::kSpecificationVersion; return aEncoder.Encode(specification_version); } diff --git a/src/protocols/secure_channel/BUILD.gn b/src/protocols/secure_channel/BUILD.gn index dcae65a6f4d271..1202c0bb88a9ae 100644 --- a/src/protocols/secure_channel/BUILD.gn +++ b/src/protocols/secure_channel/BUILD.gn @@ -88,5 +88,5 @@ static_library("secure_channel") { "${chip_root}/src/transport", ] - deps = [ "${chip_root}/src/app:revision_info" ] + deps = [ "${chip_root}/src/app:constants" ] } diff --git a/src/protocols/secure_channel/PairingSession.cpp b/src/protocols/secure_channel/PairingSession.cpp index 23daea30f2800c..80e048597578be 100644 --- a/src/protocols/secure_channel/PairingSession.cpp +++ b/src/protocols/secure_channel/PairingSession.cpp @@ -18,9 +18,7 @@ #include -#include -#include -#include +#include #include #include #include @@ -112,13 +110,13 @@ CHIP_ERROR PairingSession::EncodeSessionParameters(TLV::Tag tag, const Optional< ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(SessionParameters::Tag::kSessionActiveThreshold), mrpLocalConfig.mActiveThresholdTime.count())); - uint16_t dataModel = CHIP_DEVICE_DATA_MODEL_REVISION; + uint16_t dataModel = Revision::kDataModelRevision; ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(SessionParameters::Tag::kDataModelRevision), dataModel)); - uint16_t interactionModel = CHIP_DEVICE_INTERACTION_MODEL_REVISION; + uint16_t interactionModel = Revision::kInteractionModelRevision; ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(SessionParameters::Tag::kInteractionModelRevision), interactionModel)); - uint32_t specVersion = CHIP_DEVICE_SPECIFICATION_VERSION; + uint32_t specVersion = Revision::kSpecificationVersion; ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(SessionParameters::Tag::kSpecificationVersion), specVersion)); uint16_t maxPathsPerInvoke = CHIP_CONFIG_MAX_PATHS_PER_INVOKE;