From 2b914edff2a6a6786bc8bec052446eb69aeac9f6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 09:39:29 -0400 Subject: [PATCH 01/18] Some cleanup logic for event generation - naming and return values as eventid is not the same as event number --- src/app/interaction-model/Events.h | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 255a55e97ee465..9c979205970519 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -25,6 +25,7 @@ #include #include +#include namespace chip { namespace app { @@ -45,8 +46,8 @@ class SimpleEventLoggingDelegate : public EventLoggingDelegate const T & mEventData; }; -template ::value, bool> = true> -EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId aEndpoint) +template ::value, bool> = true> +std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId aEndpoint) { internal::SimpleEventLoggingDelegate eventData(aEventData); ConcreteEventPath path(aEndpoint, aEventData.GetClusterId(), aEventData.GetEventId()); @@ -55,12 +56,11 @@ EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId aEndpoin eventOptions.mPriority = aEventData.GetPriorityLevel(); eventOptions.mFabricIndex = aEventData.GetFabricIndex(); - // this skips logging the event if it's fabric-scoped but no fabric association exists yet. - + // this skips generating the event if it's fabric-scoped but no fabric association exists yet. if (eventOptions.mFabricIndex == kUndefinedFabricIndex) { ChipLogError(EventLogging, "Event encode failure: no fabric index for fabric scoped event"); - return kInvalidEventId; + return std::nullopt; } // @@ -72,18 +72,18 @@ EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId aEndpoin // and used to match against the accessing fabric. // EventNumber eventNumber; - CHIP_ERROR err = emittor.GenerateEvent(&eventData, eventOptions, eventNumber); + CHIP_ERROR err = generator.GenerateEvent(&eventData, eventOptions, eventNumber); if (err != CHIP_NO_ERROR) { ChipLogError(EventLogging, "Failed to log event: %" CHIP_ERROR_FORMAT, err.Format()); - return kInvalidEventId; + return std::nullopt; } return eventNumber; } -template ::value, bool> = true> -EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId endpointId) +template ::value, bool> = true> +std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId endpointId) { internal::SimpleEventLoggingDelegate eventData(aEventData); ConcreteEventPath path(endpointId, aEventData.GetClusterId(), aEventData.GetEventId()); @@ -91,11 +91,11 @@ EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId endpoint eventOptions.mPath = path; eventOptions.mPriority = aEventData.GetPriorityLevel(); EventNumber eventNumber; - CHIP_ERROR err = emittor.GenerateEvent(&eventData, eventOptions, eventNumber); + CHIP_ERROR err = generator.GenerateEvent(&eventData, eventOptions, eventNumber); if (err != CHIP_NO_ERROR) { ChipLogError(EventLogging, "Failed to log event: %" CHIP_ERROR_FORMAT, err.Format()); - return kInvalidEventId; + return std::nullopt; } return eventNumber; @@ -117,9 +117,10 @@ class Events EventNumber & generatedEventNumber) = 0; // Convenience methods for event logging using cluster-object structures - // On error, these log and return kInvalidEventId + // + // On error, these log and return nullopt. template - EventNumber GenerateEvent(const T & eventData, EndpointId endpointId) + std::optional GenerateEvent(const T & eventData, EndpointId endpointId) { return internal::GenerateEvent(*this, eventData, endpointId); } From 4d66cd1bf1fb95bf34f1b6b8d6851c50891fc2ee Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 09:40:57 -0400 Subject: [PATCH 02/18] Comment fix --- src/app/interaction-model/Events.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 9c979205970519..15194863a8554b 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -56,7 +56,8 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En eventOptions.mPriority = aEventData.GetPriorityLevel(); eventOptions.mFabricIndex = aEventData.GetFabricIndex(); - // this skips generating the event if it's fabric-scoped but no fabric association exists yet. + // this skips generating the event if it is fabric-scoped however the event does not seem + // associated with any fabric. if (eventOptions.mFabricIndex == kUndefinedFabricIndex) { ChipLogError(EventLogging, "Event encode failure: no fabric index for fabric scoped event"); From 0ae3b5c9070d992d4567ce034b19a8a0250e039f Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 09:44:33 -0400 Subject: [PATCH 03/18] More naming updates --- src/app/interaction-model/Events.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 15194863a8554b..86e121c9e9a327 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -24,8 +24,8 @@ #include #include -#include #include +#include namespace chip { namespace app { @@ -49,7 +49,7 @@ class SimpleEventLoggingDelegate : public EventLoggingDelegate template ::value, bool> = true> std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId aEndpoint) { - internal::SimpleEventLoggingDelegate eventData(aEventData); + internal::SimpleEventLoggingDelegate eventPayloadWriter(aEventData); ConcreteEventPath path(aEndpoint, aEventData.GetClusterId(), aEventData.GetEventId()); EventOptions eventOptions; eventOptions.mPath = path; @@ -73,10 +73,10 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En // and used to match against the accessing fabric. // EventNumber eventNumber; - CHIP_ERROR err = generator.GenerateEvent(&eventData, eventOptions, eventNumber); + CHIP_ERROR err = generator.GenerateEvent(&eventPayloadWriter, eventOptions, eventNumber); if (err != CHIP_NO_ERROR) { - ChipLogError(EventLogging, "Failed to log event: %" CHIP_ERROR_FORMAT, err.Format()); + ChipLogError(EventLogging, "Failed to generate event: %" CHIP_ERROR_FORMAT, err.Format()); return std::nullopt; } @@ -86,16 +86,16 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En template ::value, bool> = true> std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId endpointId) { - internal::SimpleEventLoggingDelegate eventData(aEventData); + internal::SimpleEventLoggingDelegate eventPayloadWriter(aEventData); ConcreteEventPath path(endpointId, aEventData.GetClusterId(), aEventData.GetEventId()); EventOptions eventOptions; eventOptions.mPath = path; eventOptions.mPriority = aEventData.GetPriorityLevel(); EventNumber eventNumber; - CHIP_ERROR err = generator.GenerateEvent(&eventData, eventOptions, eventNumber); + CHIP_ERROR err = generator.GenerateEvent(&eventPayloadWriter, eventOptions, eventNumber); if (err != CHIP_NO_ERROR) { - ChipLogError(EventLogging, "Failed to log event: %" CHIP_ERROR_FORMAT, err.Format()); + ChipLogError(EventLogging, "Failed to generate event: %" CHIP_ERROR_FORMAT, err.Format()); return std::nullopt; } @@ -114,11 +114,11 @@ class Events /// Events are generally expected to be sent to subscribed clients and also /// be available for read later until they get overwritten by new events /// that are being generated. - virtual CHIP_ERROR GenerateEvent(EventLoggingDelegate * eventContentWriter, const EventOptions & options, + virtual CHIP_ERROR GenerateEvent(EventLoggingDelegate * eventPayloadWriter, const EventOptions & options, EventNumber & generatedEventNumber) = 0; // Convenience methods for event logging using cluster-object structures - // + // // On error, these log and return nullopt. template std::optional GenerateEvent(const T & eventData, EndpointId endpointId) From 9cbabe171b9b9e4d8bd833d1d2517320bba5e93d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:13:49 -0400 Subject: [PATCH 04/18] Several comment updates and renamed RequestContext to ActionContext --- .../{RequestContext.h => ActionContext.h} | 8 ++++---- src/app/interaction-model/Actions.h | 4 ++-- src/app/interaction-model/BUILD.gn | 2 +- src/app/interaction-model/InvokeResponder.h | 2 +- src/app/interaction-model/IterationTypes.h | 9 +++++++-- src/app/interaction-model/Model.h | 10 +++++----- src/app/interaction-model/OperationTypes.h | 4 ++-- src/app/interaction-model/Paths.h | 16 +++++++--------- 8 files changed, 29 insertions(+), 26 deletions(-) rename src/app/interaction-model/{RequestContext.h => ActionContext.h} (87%) diff --git a/src/app/interaction-model/RequestContext.h b/src/app/interaction-model/ActionContext.h similarity index 87% rename from src/app/interaction-model/RequestContext.h rename to src/app/interaction-model/ActionContext.h index 74fa9af9da0fc8..bfc2555870c780 100644 --- a/src/app/interaction-model/RequestContext.h +++ b/src/app/interaction-model/ActionContext.h @@ -22,13 +22,13 @@ namespace chip { namespace app { namespace InteractionModel { -// Context for a currently executing request -class RequestContext +// Context for a currently executing action +class ActionContext { public: - virtual ~RequestContext() = default; + virtual ~ActionContext() = default; - /// Valid ONLY during synchronous handling of a Read/Write/Invoke + /// Valid ONLY during synchronous handling of an action. /// /// Used sparingly, however some operations will require these. An example /// usage is "Operational Credentials aborting communications on removed fabrics" diff --git a/src/app/interaction-model/Actions.h b/src/app/interaction-model/Actions.h index 62021fbf7ccd32..51574f3596b935 100644 --- a/src/app/interaction-model/Actions.h +++ b/src/app/interaction-model/Actions.h @@ -18,7 +18,7 @@ #include #include -#include +#include namespace chip { namespace app { @@ -29,7 +29,7 @@ struct InteractionModelActions { Events * events; Paths * paths; - RequestContext * requestContext; + ActionContext * requestContext; }; } // namespace InteractionModel diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index c91c2aedac633e..7525fee12057f5 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -16,13 +16,13 @@ import("//build_overrides/chip.gni") source_set("interaction-model") { sources = [ "Actions.h", + "ActionContex.h", "Events.h", "InvokeResponder.h", "IterationTypes.h", "Model.h", "OperationTypes.h", "Paths.h", - "RequestContext.h", ] public_deps = [ diff --git a/src/app/interaction-model/InvokeResponder.h b/src/app/interaction-model/InvokeResponder.h index 0a399b24564e96..54ae9ba0f51a08 100644 --- a/src/app/interaction-model/InvokeResponder.h +++ b/src/app/interaction-model/InvokeResponder.h @@ -26,7 +26,7 @@ namespace InteractionModel { /// Handles encoding of an invoke response for a specific invoke request. /// -/// This class handles a single response (i.e. a CommandDataIB within the +/// This class handles a single request (i.e. a CommandDataIB within the /// matter protocol) and is responsible for constructing its corresponding /// response (i.e. a InvokeResponseIB within the matter protocol) /// diff --git a/src/app/interaction-model/IterationTypes.h b/src/app/interaction-model/IterationTypes.h index 32ad8bc42b73be..5accbf63fcd652 100644 --- a/src/app/interaction-model/IterationTypes.h +++ b/src/app/interaction-model/IterationTypes.h @@ -70,8 +70,13 @@ struct AttributeEntry /// Iteration rules: /// - kInvalidEndpointId will be returned when iteration ends (or generally kInvalid* for paths) /// - Any internal iteration errors are just logged (callers do not handle iteration CHIP_ERROR) -/// - Iteration order is NOT guaranteed, however uniqueness and completeness is (must iterate -/// over all possible distinct values as long as no internal structural changes occur) +/// - Iteration order is NOT guaranteed globally. Only the following is guaranteed: +/// - when iterating over an endpoint, ALL clusters of that endpoint will be iterated first, before +/// switching the endpoint (order of clusters ids themselves not guaranteed) +/// - when iterating over a cluster, ALL attributes of that cluster will be iterated first, before +/// switching to a new cluster +/// - uniqueness and completeness (iterate over all possible distinct values as long as no +/// internal structural changes occur) class AttributeTreeIterator { public: diff --git a/src/app/interaction-model/Model.h b/src/app/interaction-model/Model.h index b3a127c074aaa1..02d8a647e15871 100644 --- a/src/app/interaction-model/Model.h +++ b/src/app/interaction-model/Model.h @@ -53,7 +53,7 @@ class Model : public AttributeTreeIterator // During the transition phase, we expect a large subset of code to require access to // event emitting, path marking and other operations - virtual InteractionModelActions CurrentActions() { return mActions; } + virtual InteractionModelActions CurrentActions() const { return mActions; } /// List reading has specific handling logic: /// `state` contains in/out data about the current list reading. MUST start with kInvalidListIndex on first call @@ -94,14 +94,14 @@ class Model : public AttributeTreeIterator /// - `NeedsTimedInteraction` for writes that are not timed however are required to be so virtual CHIP_ERROR WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) = 0; - /// `responder` is used to send back the reply. + /// `reply` is used to send back the reply. /// - calling Reply() or ReplyAsync() will let the application control the reply /// - returning a CHIP_NO_ERROR without reply/reply_async implies a Status::Success reply without data - /// - returning a CHIP_*_ERROR implies an error reply (error and data are mutually exclusive) + /// - returning a value other than CHIP_NO_ERROR implies an error reply (error and data are mutually exclusive) /// /// See InvokeReply/AutoCompleteInvokeResponder for details on how to send back replies and expected - /// error handling. If you require knowledge if a response was successfully sent, use the underlying - /// `reply` object instead of returning an error codes from Invoke. + /// error handling. If you need to know weather a response was successfully sent, use the underlying + /// `reply` object instead of returning an error code from Invoke. /// /// Return codes /// CHIP_IM_GLOBAL_STATUS(code): diff --git a/src/app/interaction-model/OperationTypes.h b/src/app/interaction-model/OperationTypes.h index 115dc11a1ff3bc..640f183ef5ccff 100644 --- a/src/app/interaction-model/OperationTypes.h +++ b/src/app/interaction-model/OperationTypes.h @@ -67,7 +67,7 @@ struct ReadState enum class WriteFlags : uint32_t { kTimed = 0x0001, // Received as a 2nd command after a timed invoke - kListBegin = 0x0002, // This is the FIRST list data element in a series of data + kListBegin = 0x0002, // This is the FIRST list of data elements kListEnd = 0x0004, // This is the LAST list element to write }; @@ -79,7 +79,7 @@ struct WriteAttributeRequest : OperationRequest enum class InvokeFlags : uint32_t { - kTimed = 0x0001, // Received as a 2nd command after a timed invoke + kTimed = 0x0001, // Command received as part of a timed invoke interaction. }; struct InvokeRequest : OperationRequest diff --git a/src/app/interaction-model/Paths.h b/src/app/interaction-model/Paths.h index 2bf9f0c4158011..2cf6937bb4f250 100644 --- a/src/app/interaction-model/Paths.h +++ b/src/app/interaction-model/Paths.h @@ -22,23 +22,21 @@ namespace chip { namespace app { namespace InteractionModel { -/// Handles path attributes for interaction models. +/// Notification listener for attribute changes. /// -/// It allows a user of the class to mark specific paths -/// as having changed. The intended use is for some listener to -/// perform operations as a result of something having changed, -/// usually by forwarding updates (e.g. in case of subscriptions -/// that cover that path). +/// Used to notify that a specific attribute path (or several attributes +/// via wildcards) have changed their underlying content. /// -/// Methods on this class MUCH be called from within the matter +/// Methods on this class MUST be called from within the matter /// main loop as they will likely trigger interaction model -/// internal updates and subscription event updates. +/// internal updates and subscription data reporting. class Paths { public: virtual ~Paths() = 0; - /// Mark some specific attributes dirty. + /// Mark some specific attribute dirty (or several of attributes when using wildcards). + /// /// Wildcards are supported. virtual void MarkDirty(const AttributePathParams & path) = 0; }; From b40bf0a13d9c87cc9ca8973d80a97f4aa0f6e0bf Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:14:24 -0400 Subject: [PATCH 05/18] Restyle --- src/app/interaction-model/Actions.h | 2 +- src/app/interaction-model/BUILD.gn | 2 +- src/app/interaction-model/Paths.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/interaction-model/Actions.h b/src/app/interaction-model/Actions.h index 51574f3596b935..830b349e3b91d9 100644 --- a/src/app/interaction-model/Actions.h +++ b/src/app/interaction-model/Actions.h @@ -16,9 +16,9 @@ */ #pragma once +#include #include #include -#include namespace chip { namespace app { diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index 7525fee12057f5..6dd4df8e4afa1e 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -15,8 +15,8 @@ import("//build_overrides/chip.gni") source_set("interaction-model") { sources = [ - "Actions.h", "ActionContex.h", + "Actions.h", "Events.h", "InvokeResponder.h", "IterationTypes.h", diff --git a/src/app/interaction-model/Paths.h b/src/app/interaction-model/Paths.h index 2cf6937bb4f250..fdc61e89a2dd7a 100644 --- a/src/app/interaction-model/Paths.h +++ b/src/app/interaction-model/Paths.h @@ -25,7 +25,7 @@ namespace InteractionModel { /// Notification listener for attribute changes. /// /// Used to notify that a specific attribute path (or several attributes -/// via wildcards) have changed their underlying content. +/// via wildcards) have changed their underlying content. /// /// Methods on this class MUST be called from within the matter /// main loop as they will likely trigger interaction model From 4648b3ce3f0ebb226ae7a7d9c8a9539802b7eebf Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:18:32 -0400 Subject: [PATCH 06/18] Rename to InteractionModelContext --- src/app/interaction-model/BUILD.gn | 2 +- src/app/interaction-model/{Actions.h => Context.h} | 8 ++++++-- src/app/interaction-model/Model.h | 10 +++++----- 3 files changed, 12 insertions(+), 8 deletions(-) rename src/app/interaction-model/{Actions.h => Context.h} (78%) diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index 6dd4df8e4afa1e..83e6fbf7df419b 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -16,7 +16,7 @@ import("//build_overrides/chip.gni") source_set("interaction-model") { sources = [ "ActionContex.h", - "Actions.h", + "Context.h", "Events.h", "InvokeResponder.h", "IterationTypes.h", diff --git a/src/app/interaction-model/Actions.h b/src/app/interaction-model/Context.h similarity index 78% rename from src/app/interaction-model/Actions.h rename to src/app/interaction-model/Context.h index 830b349e3b91d9..5f7f442664ff20 100644 --- a/src/app/interaction-model/Actions.h +++ b/src/app/interaction-model/Context.h @@ -25,11 +25,15 @@ namespace app { namespace InteractionModel { /// Data provided to data models in order to interface with the interaction model environment. -struct InteractionModelActions +/// +/// Provides callback-style functionality to notify the interaction model of changes +/// (e.g. using paths to notify of attribute data changes or events to generate events) +/// as well as fetching current state (via actionContext) +struct InteractionModelContext { Events * events; Paths * paths; - ActionContext * requestContext; + ActionContext * actionContext; }; } // namespace InteractionModel diff --git a/src/app/interaction-model/Model.h b/src/app/interaction-model/Model.h index 02d8a647e15871..291e5967c8cfa3 100644 --- a/src/app/interaction-model/Model.h +++ b/src/app/interaction-model/Model.h @@ -22,7 +22,7 @@ #include #include -#include +#include #include #include #include @@ -44,16 +44,16 @@ class Model : public AttributeTreeIterator virtual ~Model() = default; // `actions` pointers will be guaranteed valid until Shutdown is called() - virtual CHIP_ERROR Startup(InteractionModelActions actions) + virtual CHIP_ERROR Startup(InteractionModelContext actions) { - mActions = actions; + mContext = actions; return CHIP_NO_ERROR; } virtual CHIP_ERROR Shutdown() = 0; // During the transition phase, we expect a large subset of code to require access to // event emitting, path marking and other operations - virtual InteractionModelActions CurrentActions() const { return mActions; } + virtual InteractionModelContext CurrentActions() const { return mContext; } /// List reading has specific handling logic: /// `state` contains in/out data about the current list reading. MUST start with kInvalidListIndex on first call @@ -115,7 +115,7 @@ class Model : public AttributeTreeIterator virtual CHIP_ERROR Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, InvokeReply & reply) = 0; private: - InteractionModelActions mActions = { nullptr }; + InteractionModelContext mContext = { nullptr }; }; } // namespace InteractionModel From 9ec35cf99b36e45dd44fd163cd341a9839ddbf33 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:19:44 -0400 Subject: [PATCH 07/18] one more rename --- src/app/interaction-model/Events.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 86e121c9e9a327..efa72308bfee36 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -33,10 +33,10 @@ namespace InteractionModel { namespace internal { template -class SimpleEventLoggingDelegate : public EventLoggingDelegate +class SimpleEventPayloadWriter : public EventLoggingDelegate { public: - SimpleEventLoggingDelegate(const T & aEventData) : mEventData(aEventData){}; + SimpleEventPayloadWriter(const T & aEventData) : mEventData(aEventData){}; CHIP_ERROR WriteEvent(chip::TLV::TLVWriter & aWriter) final override { return DataModel::Encode(aWriter, TLV::ContextTag(EventDataIB::Tag::kData), mEventData); @@ -49,7 +49,7 @@ class SimpleEventLoggingDelegate : public EventLoggingDelegate template ::value, bool> = true> std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId aEndpoint) { - internal::SimpleEventLoggingDelegate eventPayloadWriter(aEventData); + internal::SimpleEventPayloadWriter eventPayloadWriter(aEventData); ConcreteEventPath path(aEndpoint, aEventData.GetClusterId(), aEventData.GetEventId()); EventOptions eventOptions; eventOptions.mPath = path; @@ -86,7 +86,7 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En template ::value, bool> = true> std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId endpointId) { - internal::SimpleEventLoggingDelegate eventPayloadWriter(aEventData); + internal::SimpleEventPayloadWriter eventPayloadWriter(aEventData); ConcreteEventPath path(endpointId, aEventData.GetClusterId(), aEventData.GetEventId()); EventOptions eventOptions; eventOptions.mPath = path; From 292c1c9bf2289a00f9ee188be483e1a5a05f0170 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:22:44 -0400 Subject: [PATCH 08/18] Fix typo --- src/app/interaction-model/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index 83e6fbf7df419b..a0967289c6c65c 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -15,7 +15,7 @@ import("//build_overrides/chip.gni") source_set("interaction-model") { sources = [ - "ActionContex.h", + "ActionContext.h", "Context.h", "Events.h", "InvokeResponder.h", From 55c715d47e78308e1f73d63d791357c167d1b029 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:25:52 -0400 Subject: [PATCH 09/18] Fix tests to compile --- .../tests/TestEventEmitting.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/app/interaction-model/tests/TestEventEmitting.cpp b/src/app/interaction-model/tests/TestEventEmitting.cpp index cb49dc25caa068..2bf9661fd27809 100644 --- a/src/app/interaction-model/tests/TestEventEmitting.cpp +++ b/src/app/interaction-model/tests/TestEventEmitting.cpp @@ -100,8 +100,10 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) StartUpEventType event{ kFakeSoftwareVersion }; - EventNumber n1 = events->GenerateEvent(event, 0 /* EndpointId */); - ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); + std::optional n1 = events->GenerateEvent(event, 0 /* EndpointId */); + + ASSERT_TRUE(n1.has_value()); + ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId())); @@ -115,8 +117,9 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) ASSERT_EQ(err, CHIP_NO_ERROR); ASSERT_EQ(decoded_event.softwareVersion, kFakeSoftwareVersion); - EventNumber n2 = events->GenerateEvent(event, /* endpointId = */ 1); - ASSERT_EQ(n2, logOnlyEvents.CurrentEventNumber()); + std::optional n2 = events->GenerateEvent(event, /* endpointId = */ 1); + ASSERT_TRUE(n2.has_value()); + ASSERT_EQ(*n2, logOnlyEvents.CurrentEventNumber()); ASSERT_NE(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, @@ -137,14 +140,14 @@ TEST(TestInteractionModelEventEmitting, TestFabricScoped) event.adminNodeID = chip::app::DataModel::MakeNullable(kTestNodeId); event.adminPasscodeID = chip::app::DataModel::MakeNullable(kTestPasscode); - EventNumber n1 = events->GenerateEvent(event, 0 /* EndpointId */); + std::optional n1 = events->GenerateEvent(event, 0 /* EndpointId */); // encoding without a fabric ID MUST fail for fabric events - ASSERT_EQ(n1, kInvalidEventId); + ASSERT_FALSE(n1.has_value()); event.fabricIndex = kTestFabricIndex; n1 = events->GenerateEvent(event, /* endpointId = */ 0); - ASSERT_NE(n1, kInvalidEventId); + ASSERT_TRUE(n1.has_value()); ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, AccessControlEntryChangedType::GetClusterId(), From d34e51d1e20ea78d33ef85fd3a02b85cec980f86 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:29:04 -0400 Subject: [PATCH 10/18] More renames of actions to context --- src/app/interaction-model/Model.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/interaction-model/Model.h b/src/app/interaction-model/Model.h index 291e5967c8cfa3..151065f4c540d2 100644 --- a/src/app/interaction-model/Model.h +++ b/src/app/interaction-model/Model.h @@ -43,17 +43,17 @@ class Model : public AttributeTreeIterator public: virtual ~Model() = default; - // `actions` pointers will be guaranteed valid until Shutdown is called() - virtual CHIP_ERROR Startup(InteractionModelContext actions) + // `context` pointers will be guaranteed valid until Shutdown is called() + virtual CHIP_ERROR Startup(InteractionModelContext context) { - mContext = actions; + mContext = context; return CHIP_NO_ERROR; } virtual CHIP_ERROR Shutdown() = 0; // During the transition phase, we expect a large subset of code to require access to // event emitting, path marking and other operations - virtual InteractionModelContext CurrentActions() const { return mContext; } + virtual InteractionModelContext CurrentContext() const { return mContext; } /// List reading has specific handling logic: /// `state` contains in/out data about the current list reading. MUST start with kInvalidListIndex on first call From 5332c0b0d5a2a91edd44ce1518b500ca383a46cf Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:42:38 -0400 Subject: [PATCH 11/18] One more comment added --- src/app/interaction-model/Events.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index efa72308bfee36..f127f214c592ab 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -104,6 +104,10 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En } // namespace internal +/// Exposes event access capabilities. +/// +/// Allows callers to "generate events" which effectively notifies of an event having +/// ocurred. class Events { public: From b95086cb16c489195bcc71c178cff719a6ae85d2 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 11:39:38 -0400 Subject: [PATCH 12/18] Restyle --- src/app/interaction-model/Events.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index f127f214c592ab..87c479d831fd71 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -107,7 +107,7 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En /// Exposes event access capabilities. /// /// Allows callers to "generate events" which effectively notifies of an event having -/// ocurred. +/// ocurred. class Events { public: From 81e268ddd1d91e0c1e40defec0c00abe579ee55d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 13:09:22 -0400 Subject: [PATCH 13/18] make clang-tidy happy --- src/app/interaction-model/tests/TestEventEmitting.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/interaction-model/tests/TestEventEmitting.cpp b/src/app/interaction-model/tests/TestEventEmitting.cpp index 2bf9661fd27809..31632fbfece2bc 100644 --- a/src/app/interaction-model/tests/TestEventEmitting.cpp +++ b/src/app/interaction-model/tests/TestEventEmitting.cpp @@ -103,7 +103,7 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) std::optional n1 = events->GenerateEvent(event, 0 /* EndpointId */); ASSERT_TRUE(n1.has_value()); - ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); + ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId())); @@ -119,7 +119,7 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) std::optional n2 = events->GenerateEvent(event, /* endpointId = */ 1); ASSERT_TRUE(n2.has_value()); - ASSERT_EQ(*n2, logOnlyEvents.CurrentEventNumber()); + ASSERT_EQ(*n2, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) ASSERT_NE(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, @@ -148,7 +148,7 @@ TEST(TestInteractionModelEventEmitting, TestFabricScoped) n1 = events->GenerateEvent(event, /* endpointId = */ 0); ASSERT_TRUE(n1.has_value()); - ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); + ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, AccessControlEntryChangedType::GetClusterId(), AccessControlEntryChangedType::GetEventId())); From 2f548a6849a5e2370e6053963d8c3b08158f2f0c Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 13:11:41 -0400 Subject: [PATCH 14/18] Operator== exists on optional ... make use of that directly --- src/app/interaction-model/tests/TestEventEmitting.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/app/interaction-model/tests/TestEventEmitting.cpp b/src/app/interaction-model/tests/TestEventEmitting.cpp index 31632fbfece2bc..3196636f621653 100644 --- a/src/app/interaction-model/tests/TestEventEmitting.cpp +++ b/src/app/interaction-model/tests/TestEventEmitting.cpp @@ -102,8 +102,7 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) std::optional n1 = events->GenerateEvent(event, 0 /* EndpointId */); - ASSERT_TRUE(n1.has_value()); - ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) + ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId())); @@ -118,9 +117,7 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) ASSERT_EQ(decoded_event.softwareVersion, kFakeSoftwareVersion); std::optional n2 = events->GenerateEvent(event, /* endpointId = */ 1); - ASSERT_TRUE(n2.has_value()); - ASSERT_EQ(*n2, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) - ASSERT_NE(n1, logOnlyEvents.CurrentEventNumber()); + ASSERT_EQ(n2, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(1 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId())); @@ -147,8 +144,7 @@ TEST(TestInteractionModelEventEmitting, TestFabricScoped) event.fabricIndex = kTestFabricIndex; n1 = events->GenerateEvent(event, /* endpointId = */ 0); - ASSERT_TRUE(n1.has_value()); - ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) + ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, AccessControlEntryChangedType::GetClusterId(), AccessControlEntryChangedType::GetEventId())); From 2b32cd4cc232e7c97c4b6a217e2d4e6e53238c8d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:24:15 -0400 Subject: [PATCH 15/18] Update src/app/interaction-model/Events.h Co-authored-by: Boris Zbarsky --- src/app/interaction-model/Events.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 87c479d831fd71..0fc3fc56b974bd 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -56,7 +56,7 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En eventOptions.mPriority = aEventData.GetPriorityLevel(); eventOptions.mFabricIndex = aEventData.GetFabricIndex(); - // this skips generating the event if it is fabric-scoped however the event does not seem + // this skips generating the event if it is fabric-scoped but the provided event data is not // associated with any fabric. if (eventOptions.mFabricIndex == kUndefinedFabricIndex) { From a9f5680e1ac7d5213be35c38fc3d7589563361ce Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:25:33 -0400 Subject: [PATCH 16/18] Update src/app/interaction-model/IterationTypes.h Co-authored-by: Boris Zbarsky --- src/app/interaction-model/IterationTypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/IterationTypes.h b/src/app/interaction-model/IterationTypes.h index 5accbf63fcd652..441dd3acb7b81f 100644 --- a/src/app/interaction-model/IterationTypes.h +++ b/src/app/interaction-model/IterationTypes.h @@ -72,7 +72,7 @@ struct AttributeEntry /// - Any internal iteration errors are just logged (callers do not handle iteration CHIP_ERROR) /// - Iteration order is NOT guaranteed globally. Only the following is guaranteed: /// - when iterating over an endpoint, ALL clusters of that endpoint will be iterated first, before -/// switching the endpoint (order of clusters ids themselves not guaranteed) +/// switching the endpoint (order of clusters themselves not guaranteed) /// - when iterating over a cluster, ALL attributes of that cluster will be iterated first, before /// switching to a new cluster /// - uniqueness and completeness (iterate over all possible distinct values as long as no From b1f6bebf651f802cec1c9c5a99a487b98a39a874 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:25:42 -0400 Subject: [PATCH 17/18] Update src/app/interaction-model/Paths.h Co-authored-by: Boris Zbarsky --- src/app/interaction-model/Paths.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/Paths.h b/src/app/interaction-model/Paths.h index fdc61e89a2dd7a..9241222a990cb5 100644 --- a/src/app/interaction-model/Paths.h +++ b/src/app/interaction-model/Paths.h @@ -35,7 +35,7 @@ class Paths public: virtual ~Paths() = 0; - /// Mark some specific attribute dirty (or several of attributes when using wildcards). + /// Mark all attributes matching the given path (which may be a wildcard) dirty. /// /// Wildcards are supported. virtual void MarkDirty(const AttributePathParams & path) = 0; From 1e0314358ec8828052118904b740a0da4c627ac5 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 16:51:41 -0400 Subject: [PATCH 18/18] Comment update --- src/app/interaction-model/OperationTypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/OperationTypes.h b/src/app/interaction-model/OperationTypes.h index 640f183ef5ccff..57499a8dbafb49 100644 --- a/src/app/interaction-model/OperationTypes.h +++ b/src/app/interaction-model/OperationTypes.h @@ -66,7 +66,7 @@ struct ReadState enum class WriteFlags : uint32_t { - kTimed = 0x0001, // Received as a 2nd command after a timed invoke + kTimed = 0x0001, // Write is a timed write (i.e. a Timed Request Action preceeded it) kListBegin = 0x0002, // This is the FIRST list of data elements kListEnd = 0x0004, // This is the LAST list element to write };