Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

[Matter.framework] Move various #define to src/platform/Darwin/CHIPPl… #36178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+38 −139
Open

[Matter.framework] Move various #define to src/platform/Darwin/CHIPPl… #36178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions config/ios/args.gni → config/darwin/args.gni
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2020 Project CHIP Authors
# Copyright (c) 2024 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.
Expand All @@ -14,9 +14,9 @@

import("//build_overrides/chip.gni")

chip_system_config_clock = "gettimeofday"
chip_device_platform = "darwin"

chip_project_config_include_dirs = [ "${chip_root}/config/ios" ]
chip_project_config_include = ""
chip_system_project_config_include = ""

pw_build_PIP_CONSTRAINTS = [ "${chip_root}/scripts/setup/constraints.txt" ]
52 changes: 0 additions & 52 deletions config/ios/CHIPProjectConfig.h
View file
Open in desktop

This file was deleted.

26 changes: 0 additions & 26 deletions config/ios/SystemProjectConfig.h
View file
Open in desktop

This file was deleted.

13 changes: 1 addition & 12 deletions examples/darwin-framework-tool/args.gni
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,4 @@

import("//build_overrides/chip.gni")

import("${chip_root}/config/standalone/args.gni")

chip_crypto = "boringssl"

chip_device_project_config_include =
"${chip_root}/examples/darwin-framework-tool/include/CHIPProjectAppConfig.h"
chip_project_config_include =
"${chip_root}/examples/darwin-framework-tool/include/CHIPProjectAppConfig.h"

chip_project_config_include_dirs =
[ "${chip_root}/examples/darwin-framework-tool/include" ]
chip_project_config_include_dirs += [ "${chip_root}/config/standalone" ]
import("${chip_root}/config/darwin/args.gni")
31 changes: 0 additions & 31 deletions examples/darwin-framework-tool/include/CHIPProjectAppConfig.h
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ...-casting-app/darwin/MatterTvCastingBridge/MatterTvCastingBridge.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,6 @@
"$(CHIP_ROOT)/src/include",
"$(CHIP_ROOT)/src/lib",
"$(CHIP_ROOT)/src/app",
"$(CHIP_ROOT)/config/ios",
"$(CHIP_ROOT)/src/app/util",
"$(CHIP_ROOT)/third_party/nlassert/repo/include",
"$(CHIP_ROOT)/third_party/nlio/repo/include",
Expand Down Expand Up @@ -759,7 +758,6 @@
"$(CHIP_ROOT)/src/include",
"$(CHIP_ROOT)/src/lib",
"$(CHIP_ROOT)/src/app",
"$(CHIP_ROOT)/config/ios",
"$(CHIP_ROOT)/src/app/util",
"$(CHIP_ROOT)/third_party/nlassert/repo/include",
"$(CHIP_ROOT)/third_party/nlio/repo/include",
Expand Down
7 changes: 2 additions & 5 deletions examples/tv-casting-app/darwin/args.gni
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,14 @@

import("//build_overrides/chip.gni")

import("${chip_root}/config/ios/args.gni")
import("${chip_root}/config/darwin/args.gni")

chip_device_project_config_include = "<CHIPProjectAppConfig.h>"
chip_project_config_include = "<CHIPProjectAppConfig.h>"
chip_system_project_config_include = "<SystemProjectConfig.h>"

chip_project_config_include_dirs += [
chip_project_config_include_dirs = [
vivien-apple marked this conversation as resolved.
Show resolved Hide resolved
"${chip_root}/examples/tv-casting-app/tv-casting-common/include",
"${chip_root}/examples/tv-casting-app/tv-casting-common",
]
chip_project_config_include_dirs += [ "${chip_root}/config/ios" ]

chip_build_libshell = true

Expand Down
2 changes: 2 additions & 0 deletions examples/tv-casting-app/tv-casting-common/include/CHIPProjectAppConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,6 @@

// Include the CHIPProjectConfig from config/standalone
// Add this at the end so that we can hit our #defines first
#ifndef CHIP_DEVICE_LAYER_TARGET_DARWIN
#include <CHIPProjectConfig.h>
#endif // CHIP_DEVICE_LAYER_TARGET_DARWIN
4 changes: 0 additions & 4 deletions src/darwin/Framework/Matter.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2257,7 +2257,6 @@
"$(CHIP_ROOT)/third_party/nlio/repo/include",
"$(CHIP_ROOT)/third_party/jsoncpp/repo/include",
"$(CHIP_ROOT)/zzz_generated/darwin-framework-tool",
"$(CHIP_ROOT)/config/ios",
"$(CHIP_ROOT)/third_party/editline/repo/include",
"$(CHIP_ROOT)/src/include",
"$(CONFIGURATION_TEMP_DIR)/Matter.build/out/gen/include",
Expand Down Expand Up @@ -2334,7 +2333,6 @@
"$(CHIP_ROOT)/zzz_generated/app-common",
"$(CHIP_ROOT)/third_party/nlio/repo/include",
"$(CHIP_ROOT)/third_party/jsoncpp/repo/include",
"$(CHIP_ROOT)/config/ios",
"$(CHIP_ROOT)/third_party/editline/repo/include",
"$(CHIP_ROOT)/third_party/libwebsockets",
"$(CHIP_ROOT)/third_party/libwebsockets/repo/include",
Expand Down Expand Up @@ -2526,7 +2524,6 @@
SYSTEM_HEADER_SEARCH_PATHS = (
"$(TEMP_DIR)/out/gen/include",
"$(CHIP_ROOT)/src/darwin/Framework/CHIP/",
"$(CHIP_ROOT)/config/ios",
"$(CHIP_ROOT)/src",
"$(CHIP_ROOT)/src/include",
"$(CHIP_ROOT)/zzz_generated/",
Expand Down Expand Up @@ -2698,7 +2695,6 @@
SYSTEM_HEADER_SEARCH_PATHS = (
"$(TEMP_DIR)/out/gen/include",
"$(CHIP_ROOT)/src/darwin/Framework/CHIP/",
"$(CHIP_ROOT)/config/ios",
"$(CHIP_ROOT)/src",
"$(CHIP_ROOT)/src/include",
"$(CHIP_ROOT)/zzz_generated/",
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/chip_xcode_build_connector.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ done
[[ $ENABLE_BITCODE == YES ]] && target_cflags+=("-flto")

declare -a args=(
'import("//config/darwin/args.gni")'
'default_configs_cosmetic=[]' # suppress colorization
'chip_crypto="boringssl"'
'chip_build_controller_dynamic_server=false'
Expand Down Expand Up @@ -132,7 +133,6 @@ esac
[[ $PLATFORM_FAMILY_NAME != macOS ]] && {
args+=(
'target_os="ios"'
'import("//config/ios/args.gni")'
)
}

Expand Down
28 changes: 25 additions & 3 deletions src/platform/Darwin/CHIPPlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

#pragma once

#include <TargetConditionals.h>

// ==================== General Platform Adaptations ====================

#define CHIP_CONFIG_ABORT() abort()
Expand Down Expand Up @@ -53,9 +55,16 @@ extern "C" int __cxa_atexit(void (*f)(void *), void * p, void * d);
#define CHIP_CONFIG_MAX_UNSOLICITED_MESSAGE_HANDLERS 8
#endif // CHIP_CONFIG_MAX_UNSOLICITED_MESSAGE_HANDLERS

#ifndef CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS
#define CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS 8
#endif // CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS
//
// Default of 8 ECs is not sufficient for some of the unit tests
// that try to validate multiple simultaneous interactions.
Comment on lines +59 to +60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should only matter when building those unit tests, right?

That said, in general we might in fact want more than 8 on darwin.....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We define CHIP_SYSTEM_CONFIG_POOL_USE_HEAP = 1 so the pool size should be entirely irrelevant according to Pool.h:

template <typename T, size_t N>
class ObjectPool<T, N, ObjectPoolMem::kHeap> : public HeapObjectPool<T> {};

So I think we should just not override CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this macro is being used to size an array as well.

from src/lib/core/CHIPConfig.h

#define CHIP_CONFIG_MCSP_RECEIVE_TABLE_SIZE (CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS - 2)

from src/protocols/secure_channel/MessageCounterManager.h:

ReceiveTableEntry mReceiveTable[CHIP_CONFIG_MCSP_RECEIVE_TABLE_SIZE];

// In tests like TestReadHandler_MultipleSubscriptions, we are trying to issue as many read / subscription requests as possible in
// parallel. Since the default config says we support 16 fabrics, and we will have 4 read handlers for each fabric (3 subscriptions
// + 1 reserved for read) that is read transactions in parallel. Since the report handlers are allocated on the heap, we will issue
// 65 requests (the TestReadHandler_MultipleSubscriptions will issue CHIP_IM_MAX_NUM_READ_HANDLER + 1 subscriptions to verify heap
// allocation logic) in total and that is 130 ECs. Round this up to 150 ECs
//
#define CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS 150

#ifndef CHIP_LOG_FILTERING
#define CHIP_LOG_FILTERING 1
Expand All @@ -66,5 +75,18 @@ extern "C" int __cxa_atexit(void (*f)(void *), void * p, void * d);
#endif // CHIP_CONFIG_BDX_MAX_NUM_TRANSFERS

#ifndef CHIP_CONFIG_KVS_PATH
#if TARGET_OS_IPHONE
#define CHIP_CONFIG_KVS_PATH "chip.store"
Comment on lines +78 to +79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be for all the cases that are not TARGET_OS_OSX or something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does this actually take effect? The default KVS isn't used from the Framework right?
Also, what's the effect of having a relative paths here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be for all the cases that are not TARGET_OS_OSX or something?

TARGET_OS_IPHONE includes any iOS variant, including tvOS etc (it's terribly named)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does this actually take effect? The default KVS isn't used from the Framework right? Also, what's the effect of having a relative paths here?

I assumed it might be used via chip-tool.

#else
#define CHIP_CONFIG_KVS_PATH "/tmp/chip_kvs"
#endif // TARGET_OS_IPHONE
#endif // CHIP_CONFIG_KVS_PATH

#define CHIP_SYSTEM_CONFIG_PACKETBUFFER_POOL_SIZE 0

// The session pool size limits how many subscriptions we can have live at
// once. Home supports up to 1000 accessories, and we subscribe to all of them,
// so we need to make sure the pool is big enough for that.
#define CHIP_CONFIG_SECURE_SESSION_POOL_SIZE 1000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need to hard-limit the number of secure sessions in this way? SecureSessionTable::EvictAndAllocate does this rather complicated eviction logic... can we just remove the limit and compile out the eviction code entirely? (Probably something for a follow-up change)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followup at best; the secure session situation is complicated.


#define INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT 0
4 changes: 4 additions & 0 deletions src/system/system.gni
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ if (chip_system_config_locking == "") {
}
}

if (target_os == "mac" || target_os == "ios") {
chip_system_config_clock = "gettimeofday"
}

assert(
chip_system_config_locking == "posix" ||
chip_system_config_locking == "freertos" ||
Expand Down
Loading