Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove dependency of libChipController on app-specific codegen. #24555

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions examples/tv-casting-app/tv-casting-common/include/MediaBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ class MediaBase
{
public:
MediaClusterBase(chip::Messaging::ExchangeManager & exchangeManager, const chip::SessionHandle & session,
chip::ClusterId cluster, chip::EndpointId endpoint) :
ClusterBase(exchangeManager, session, cluster, endpoint)
chip::EndpointId endpoint) :
ClusterBase(exchangeManager, session, endpoint)
{}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ class MediaCommandBase : public MediaBase

sResponseCallback = responseCallback;

MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mClusterId,
mTvEndpoint);
MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mTvEndpoint);
return cluster.InvokeCommand(request, nullptr, OnSuccess, OnFailure);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ class MediaReadBase : public MediaBase
auto deviceProxy = mTargetVideoPlayerInfo->GetOperationalDeviceProxy();
ReturnErrorCodeIf(deviceProxy == nullptr || !deviceProxy->ConnectionReady(), CHIP_ERROR_PEER_NODE_NOT_FOUND);

MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mClusterId,
mTvEndpoint);
MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mTvEndpoint);

return cluster.template ReadAttribute<TypeInfo>(context, successFn, failureFn);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ class MediaSubscriptionBase : public MediaBase
auto deviceProxy = mTargetVideoPlayerInfo->GetOperationalDeviceProxy();
ReturnErrorCodeIf(deviceProxy == nullptr || !deviceProxy->ConnectionReady(), CHIP_ERROR_PEER_NODE_NOT_FOUND);

MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mClusterId,
mTvEndpoint);
MediaClusterBase cluster(*deviceProxy->GetExchangeManager(), deviceProxy->GetSecureSession().Value(), mTvEndpoint);

return cluster.template SubscribeAttribute<TypeInfo>(context, successFn, failureFn, minInterval, maxInterval,
onSubscriptionEstablished, nullptr /* resubscribeCb */,
Expand Down
7 changes: 2 additions & 5 deletions scripts/codepregen.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def _ParallelGenerateOne(arg):
@click.option(
'--generator',
default='all',
type=click.Choice(['all', 'zap', 'codegen', 'codegen-cpp-only']),
type=click.Choice(['all', 'zap', 'codegen']),
help='To what code generator to restrict the generation.')
@click.option(
'--input-glob',
Expand Down Expand Up @@ -119,12 +119,9 @@ def main(log_level, parallel, dry_run, generator, input_glob, sdk_root, output_d

if generator == 'zap':
filter.file_type = IdlFileType.ZAP
elif generator == 'codegen' or generator == 'codegen-cpp-only':
elif generator == 'codegen':
filter.file_type = IdlFileType.MATTER

if generator == 'codegen-cpp-only':
filter.cpp_only = True

targets = FindPregenerationTargets(sdk_root, filter, runner)

runner.ensure_directory_exists(output_dir)
Expand Down
4 changes: 1 addition & 3 deletions scripts/pregenerate/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ class TargetFilter:
# If non-empty only the given paths will be code-generated
path_glob: List[str] = field(default_factory=list)

cpp_only: bool = False


# TODO: the build GlobMatcher is more complete by supporting `{}` grouping
# For now this limited glob seems sufficient.
Expand Down Expand Up @@ -100,5 +98,5 @@ def FindPregenerationTargets(sdk_root: str, filter: TargetFilter, runner):
continue

for generator in generators:
if generator.Accept(idl, filter.cpp_only):
if generator.Accept(idl):
yield generator.CreateTarget(idl, runner=runner)
13 changes: 5 additions & 8 deletions scripts/pregenerate/using_codegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class CodegenBridgePregenerator:
def __init__(self, sdk_root):
self.sdk_root = sdk_root

def Accept(self, idl: InputIdlFile, cpp_only: bool):
def Accept(self, idl: InputIdlFile):
# Bridge is highly specific, a single path is acceptable for dynamic
# bridge codegen
return idl.relative_path == "examples/dynamic-bridge-app/bridge-common/bridge-app.matter"
Expand All @@ -79,12 +79,9 @@ class CodegenJavaPregenerator:
def __init__(self, sdk_root):
self.sdk_root = sdk_root

def Accept(self, idl: InputIdlFile, cpp_only: bool):
if cpp_only:
return False

# Java is highly specific, a single path is acceptable for java
# codegen
def Accept(self, idl: InputIdlFile):
# Java is highly specific, a single path is acceptable for dynamic
# bridge codegen
return idl.relative_path == "src/controller/data_model/controller-clusters.matter"

def CreateTarget(self, idl: InputIdlFile, runner):
Expand All @@ -97,7 +94,7 @@ class CodegenCppAppPregenerator:
def __init__(self, sdk_root):
self.sdk_root = sdk_root

def Accept(self, idl: InputIdlFile, cpp_only: bool):
def Accept(self, idl: InputIdlFile):
if idl.file_type != IdlFileType.MATTER:
return False

Expand Down
2 changes: 1 addition & 1 deletion scripts/pregenerate/using_zap.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class ZapApplicationPregenerator:
def __init__(self, sdk_root):
self.sdk_root = sdk_root

def Accept(self, idl: InputIdlFile, cpp_only: bool):
def Accept(self, idl: InputIdlFile):
if idl.file_type != IdlFileType.ZAP:
return False

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class DLL_EXPORT OtaSoftwareUpdateProviderCluster : public ClusterBase
public:
OtaSoftwareUpdateProviderCluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session,
EndpointId endpoint) :
ClusterBase(exchangeManager, session, app::Clusters::OtaSoftwareUpdateProvider::Id, endpoint)
ClusterBase(exchangeManager, session, endpoint)
{}
~OtaSoftwareUpdateProviderCluster() {}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class DLL_EXPORT OtaSoftwareUpdateProviderCluster : public ClusterBase
public:
OtaSoftwareUpdateProviderCluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session,
EndpointId endpoint) :
ClusterBase(exchangeManager, session, app::Clusters::OtaSoftwareUpdateProvider::Id, endpoint)
ClusterBase(exchangeManager, session, endpoint)
{}
~OtaSoftwareUpdateProviderCluster() {}
};
Expand Down
43 changes: 4 additions & 39 deletions scripts/tools/zap_regen_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,41 +113,6 @@ def generate(self) -> TargetRunStats:
)


class ZAPPregenerateTarget:
def __init__(self, input_glob, output_dir, generator=None):
self.input_glob = input_glob
self.output_dir = output_dir
self.script = "./scripts/codepregen.py"
self.command = [self.script, "--input-glob", input_glob]
self.generator = generator

if generator is not None:
self.command.extend(["--generator", generator])
self.command.append(output_dir)

def distinct_output(self):
input_template = self.input_glob
if self.generator is not None:
input_template += " " + self.generator
return ZapDistinctOutput(input_template=input_template, output_directory=self.output_dir)

def log_command(self):
logging.info(" %s" % " ".join(self.command))

def generate(self) -> TargetRunStats:
logging.info("Generating target: %s" % " ".join(self.command))

generate_start = time.time()
subprocess.check_call(self.command)
generate_end = time.time()

return TargetRunStats(
generate_time=generate_end - generate_start,
config=self.script,
template=self.script,
)


def checkPythonVersion():
if sys.version_info[0] < 3:
print('Must use Python 3. Current version is ' +
Expand Down Expand Up @@ -235,10 +200,10 @@ def getGlobalTemplatesTargets():
#
# TODO: These files can be code generated at compile time, we should figure
# out a path for this codegen to not be required.
targets.append(ZAPPregenerateTarget(
"*controller-clusters*", "zzz_generated/darwin", generator="zap"))
targets.append(ZAPPregenerateTarget(
"*controller-clusters*", "zzz_generated/darwin", generator="codegen-cpp-only"))
targets.append(ZAPGenerateTarget(
'src/controller/data_model/controller-clusters.zap',
template="src/app/zap-templates/app-templates.json",
output_dir=os.path.join('zzz_generated/darwin/controller-clusters/zap-generated')))

return targets

Expand Down
2 changes: 1 addition & 1 deletion src/app/zap-templates/templates/app/CHIPClusters.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Controller {
class DLL_EXPORT {{asUpperCamelCase name}}Cluster : public ClusterBase
{
public:
{{asUpperCamelCase name}}Cluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, EndpointId endpoint) : ClusterBase(exchangeManager, session, app::Clusters::{{asUpperCamelCase name}}::Id, endpoint) {}
{{asUpperCamelCase name}}Cluster(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, EndpointId endpoint) : ClusterBase(exchangeManager, session, endpoint) {}
~{{asUpperCamelCase name}}Cluster() {}
};

Expand Down
1 change: 0 additions & 1 deletion src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <controller/CHIPDeviceController.h>
#include <credentials/CHIPCert.h>
#include <lib/support/SafeInt.h>
#include <zap-generated/CHIPClusters.h>

namespace chip {
namespace Controller {
Expand Down
13 changes: 1 addition & 12 deletions src/controller/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,7 @@ static_library("controller") {
"${chip_root}/src/transport",
]

deps = [
# NOTE:
# awkward zapgen_zapgen dependency to get access to generated headers
# but NOT to include generated CPP files (IMCommandHandler.cpp)
"${chip_root}/src/controller/data_model:data_model_zapgen_files",
"${chip_root}/src/lib/address_resolve",
]

configs += [
# Get access to zzz_generated/CHIPClusters.h
"${chip_root}/src/controller/data_model:data_model_zapgen_config",
]
deps = [ "${chip_root}/src/lib/address_resolve" ]

defines = []
}
13 changes: 4 additions & 9 deletions src/controller/CHIPCluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ using SubscriptionOnDoneCallback = std::function<void(void)>;
class DLL_EXPORT ClusterBase
{
public:
ClusterBase(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, EndpointId endpoint) :
mExchangeManager(exchangeManager), mSession(session), mEndpoint(endpoint)
{}

virtual ~ClusterBase() {}

// Temporary function to set command timeout before we move over to InvokeCommand
Expand All @@ -69,8 +73,6 @@ class DLL_EXPORT ClusterBase
*/
Optional<System::Clock::Timeout> GetCommandTimeout() { return mTimeout; }

ClusterId GetClusterId() const { return mClusterId; }

/*
* This function permits sending an invoke request using cluster objects that represent the request and response data payloads.
*
Expand Down Expand Up @@ -397,20 +399,13 @@ class DLL_EXPORT ClusterBase
}

protected:
ClusterBase(Messaging::ExchangeManager & exchangeManager, const SessionHandle & session, ClusterId cluster,
EndpointId endpoint) :
mExchangeManager(exchangeManager),
mSession(session), mClusterId(cluster), mEndpoint(endpoint)
{}

Messaging::ExchangeManager & mExchangeManager;

// Since cluster object is ephemeral, the session shall be valid during the entire lifespan, so we do not need to check the
// session existence when using it. For java and objective-c binding, the cluster object is allocated in the heap, such that we
// can't use SessionHandle here, in such case, the cluster object must be freed when the session is released.
SessionHolder mSession;

const ClusterId mClusterId;
EndpointId mEndpoint;
Optional<System::Clock::Timeout> mTimeout;
};
Expand Down
Loading