Post-merge updates for IM/DM separation

src/app/interaction-model/RequestContext.h → 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"

src/app/interaction-model/BUILD.gn

@@ -15,14 +15,14 @@ import("//build_overrides/chip.gni")
 
 source_set("interaction-model") {
  sources = [
- "Actions.h",
+ "ActionContext.h",
+ "Context.h",
  "Events.h",
  "InvokeResponder.h",
  "IterationTypes.h",
  "Model.h",
  "OperationTypes.h",
  "Paths.h",
- "RequestContext.h",
  ]
 
  public_deps = [

src/app/interaction-model/Actions.h → src/app/interaction-model/Context.h

@@ -16,20 +16,24 @@
  */
 #pragma once
 
+#include <app/interaction-model/ActionContext.h>
 #include <app/interaction-model/Events.h>
 #include <app/interaction-model/Paths.h>
-#include <app/interaction-model/RequestContext.h>
 
 namespace chip {
 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;
- RequestContext * requestContext;
+ ActionContext * actionContext;
 };
 
 } // namespace InteractionModel

src/app/interaction-model/Events.h

@@ -24,6 +24,7 @@
 #include <lib/core/CHIPError.h>
 #include <lib/support/logging/CHIPLogging.h>
 
+#include <optional>
 #include <type_traits>
 
 namespace chip {
@@ -32,10 +33,10 @@ namespace InteractionModel {
 
 namespace internal {
 template <typename T>
-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);
@@ -45,22 +46,22 @@ class SimpleEventLoggingDelegate : public EventLoggingDelegate
  const T & mEventData;
 };
 
-template <typename E, typename T, std::enable_if_t<DataModel::IsFabricScoped<T>::value, bool> = true>
-EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId aEndpoint)
+template <typename G, typename T, std::enable_if_t<DataModel::IsFabricScoped<T>::value, bool> = true>
+std::optional<EventNumber> GenerateEvent(G & generator, const T & aEventData, EndpointId aEndpoint)
 {
- internal::SimpleEventLoggingDelegate<T> eventData(aEventData);
+ internal::SimpleEventPayloadWriter<T> eventPayloadWriter(aEventData);
  ConcreteEventPath path(aEndpoint, aEventData.GetClusterId(), aEventData.GetEventId());
  EventOptions eventOptions;
  eventOptions.mPath = path;
  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 is fabric-scoped but the provided event data is not
+ // associated with any fabric.
  if (eventOptions.mFabricIndex == kUndefinedFabricIndex)
  {
  ChipLogError(EventLogging, "Event encode failure: no fabric index for fabric scoped event");
- return kInvalidEventId;
+ return std::nullopt;
  }
 
  //
@@ -72,37 +73,41 @@ 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(&eventPayloadWriter, eventOptions, eventNumber);
  if (err != CHIP_NO_ERROR)
  {
- ChipLogError(EventLogging, "Failed to log event: %" CHIP_ERROR_FORMAT, err.Format());
- return kInvalidEventId;
+ ChipLogError(EventLogging, "Failed to generate event: %" CHIP_ERROR_FORMAT, err.Format());
+ return std::nullopt;
  }
 
  return eventNumber;
 }
 
-template <typename E, typename T, std::enable_if_t<!DataModel::IsFabricScoped<T>::value, bool> = true>
-EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId endpointId)
+template <typename G, typename T, std::enable_if_t<!DataModel::IsFabricScoped<T>::value, bool> = true>
+std::optional<EventNumber> GenerateEvent(G & generator, const T & aEventData, EndpointId endpointId)
 {
- internal::SimpleEventLoggingDelegate<T> eventData(aEventData);
+ internal::SimpleEventPayloadWriter<T> eventPayloadWriter(aEventData);
  ConcreteEventPath path(endpointId, aEventData.GetClusterId(), aEventData.GetEventId());
  EventOptions eventOptions;
  eventOptions.mPath = path;
  eventOptions.mPriority = aEventData.GetPriorityLevel();
  EventNumber eventNumber;
- CHIP_ERROR err = emittor.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());
- return kInvalidEventId;
+ ChipLogError(EventLogging, "Failed to generate event: %" CHIP_ERROR_FORMAT, err.Format());
+ return std::nullopt;
  }
 
  return eventNumber;
 }
 
 } // namespace internal
 
+/// Exposes event access capabilities.
+///
+/// Allows callers to "generate events" which effectively notifies of an event having
+/// ocurred.
 class Events
 {
 public:
@@ -113,13 +118,14 @@ 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 kInvalidEventId
+ //
+ // On error, these log and return nullopt.
  template <typename T>
- EventNumber GenerateEvent(const T & eventData, EndpointId endpointId)
+ std::optional<EventNumber> GenerateEvent(const T & eventData, EndpointId endpointId)
  {
  return internal::GenerateEvent(*this, eventData, endpointId);
  }

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)
 ///

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 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:

src/app/interaction-model/Model.h

@@ -22,7 +22,7 @@
 #include <app/AttributeValueDecoder.h>
 #include <app/AttributeValueEncoder.h>
 
-#include <app/interaction-model/Actions.h>
+#include <app/interaction-model/Context.h>
 #include <app/interaction-model/InvokeResponder.h>
 #include <app/interaction-model/IterationTypes.h>
 #include <app/interaction-model/OperationTypes.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(InteractionModelActions actions)
+ // `context` pointers will be guaranteed valid until Shutdown is called()
+ virtual CHIP_ERROR Startup(InteractionModelContext context)
  {
- mActions = 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 InteractionModelActions CurrentActions() { return mActions; }
+ 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
@@ -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):
@@ -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

src/app/interaction-model/OperationTypes.h

@@ -66,8 +66,8 @@ 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
+ 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
 };
 
@@ -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

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 all attributes matching the given path (which may be a wildcard) dirty.
+ ///
  /// Wildcards are supported.
  virtual void MarkDirty(const AttributePathParams & path) = 0;
 };

src/app/interaction-model/tests/TestEventEmitting.cpp

@@ -100,7 +100,8 @@ TEST(TestInteractionModelEventEmitting, TestBasicType)
 
  StartUpEventType event{ kFakeSoftwareVersion };
 
- EventNumber n1 = events->GenerateEvent(event, 0 /* EndpointId */);
+ std::optional<EventNumber> n1 = events->GenerateEvent(event, 0 /* EndpointId */);
+
  ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber());
  ASSERT_EQ(logOnlyEvents.LastOptions().mPath,
  ConcreteEventPath(0 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId()));
@@ -115,9 +116,8 @@ TEST(TestInteractionModelEventEmitting, TestBasicType)
  ASSERT_EQ(err, CHIP_NO_ERROR);
  ASSERT_EQ(decoded_event.softwareVersion, kFakeSoftwareVersion);
 
- EventNumber n2 = events->GenerateEvent(event, /* endpointId = */ 1);
+ std::optional<EventNumber> n2 = events->GenerateEvent(event, /* endpointId = */ 1);
  ASSERT_EQ(n2, logOnlyEvents.CurrentEventNumber());
- ASSERT_NE(n1, logOnlyEvents.CurrentEventNumber());
 
  ASSERT_EQ(logOnlyEvents.LastOptions().mPath,
  ConcreteEventPath(1 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId()));
@@ -137,14 +137,13 @@ 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<EventNumber> 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_EQ(n1, logOnlyEvents.CurrentEventNumber());
  ASSERT_EQ(logOnlyEvents.LastOptions().mPath,
  ConcreteEventPath(0 /* endpointId */, AccessControlEntryChangedType::GetClusterId(),