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

Server side event support #12059

Closed
wants to merge 8 commits into from
Closed

Server side event support #12059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f35b6aa
Added the commands the test cluster to emit events
robszewczyk Nov 18, 2021
649ef9f
Initial cut at the server-side event API
robszewczyk Nov 20, 2021
52bde92
Implemented an initial example/test of the LogEvent API
robszewczyk Nov 20, 2021
c26b2da
Restyled changes
robszewczyk Nov 20, 2021
6708064
Regenerated generated files to account for a new method in the test
robszewczyk Nov 20, 2021
97e6b22
Review comments
robszewczyk Dec 2, 2021
1c5fd13
Instantiate the EventManagement module in chip::Server
robszewczyk Dec 2, 2021
da37a47
fix header
robszewczyk Dec 2, 2021
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
51 changes: 51 additions & 0 deletions src/app/EventLogging.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
*
* Copyright (c) 2021 Project CHIP Authors
* All rights reserved.
*
* 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

#include <app/ConcreteEventPath.h>
#include <app/EventLoggingDelegate.h>
#include <app/data-model/Encode.h>
#include <app/data-model/List.h> // So we can encode lists

namespace chip {
namespace app {

template <typename T>
class EventLogger : EventLoggingDelegate
{
public:
EventLogger(const T & aEventData) : mEventData(aEventData){};
CHIP_ERROR WriteEvent(chip::TLV::TLVWriter & aWriter) final override { return mEventData.Encode(aWriter, TLV::AnonymousTag); }

private:
const T & mEventData;
};

template <typename T>
CHIP_ERROR LogEvent(const T & aEventData, EndpointId aEndpoint, EventOptions aEventOptions, EventNumber & aEventNumber)
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to decide whether we're doing the EventSchema thing (which would mean endpoint, etc is included in the EventOptions) or whether we are getting the cluster and event id from the data type and passing in the endpoint like this.

Seems to me that EventSchema should just go away.

That said event priority is not statically deducible from the data type, so we will need to pass that in, via either the EventOptions or some other mechanism.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed.

I'd rather we consolidate around EventOptions, remove EventSchema (schema is poor term for it anyways) and consequently, remove mpEventSchema from EventOptions.

Copy link
Contributor

Choose a reason for hiding this comment

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

aEventNumber needs documentation. Is this an outparam? And in/out param? What purpose does it serve?

When would it be used, outside of our test command?

Copy link
Contributor

Choose a reason for hiding this comment

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

EventNumber is outParam,

Yes, we need to consolidate around EventOption, remove EventSchema, and put ConcreteEventPath as a member inside EventOption

Copy link
Contributor

Choose a reason for hiding this comment

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

EventNumber is outParam,

My point is that this should be documented, not in a github comment.

{
EventLogger<T> eventData(aEventData);
ConcreteEventPath path(aEndpoint, aEventData.GetClusterId(), aEventData.GetEventId());
// log the actual event
aEventNumber = 0;
Copy link
Contributor

@yufengwangca yufengwangca Dec 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Shall the eventNumber be an monotonically increasing ephemeral unique number?

return CHIP_NO_ERROR;
}

} // namespace app
} // namespace chip
20 changes: 19 additions & 1 deletion src/app/clusters/test-cluster-server/test-cluster-server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <app/AttributeAccessInterface.h>
#include <app/CommandHandler.h>
#include <app/ConcreteCommandPath.h>
#include <app/EventLogging.h>
#include <app/util/attribute-storage.h>
#include <lib/core/CHIPSafeCasts.h>
#include <lib/core/CHIPTLV.h>
Expand Down Expand Up @@ -353,9 +354,26 @@ bool emberAfTestClusterClusterTestListStructArgumentRequestCallback(

return SendBooleanResponse(commandObj, commandPath, shouldReturnTrue);
}
bool emberAfTestClusterClusterTestEmitTestEventRequestCallback(
CommandHandler * commandObj, const ConcreteCommandPath & commandPath,
const Commands::TestEmitTestEventRequest::DecodableType & commandData)
{
Commands::TestEmitTestEventResponse::Type responseData;
DataModel::List<const Structs::SimpleStruct::Type> arg5;
DataModel::List<const SimpleEnum> arg6;
Comment on lines +362 to +363
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not pass these in the command payload?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend using NullablesAndOptionalsStruct since it covers lists as well as nullables/optionals, and embedding that in both the command and the event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was the original pattern I was shooting for, but the interface to go from DecodableType to Type proved to be marginally more complicated, so I mocked up something temporary. Will fix.

EventOptions eventOptions;
Events::TestEvent::Type event{ commandData.arg1, commandData.arg2, commandData.arg3, commandData.arg4, arg5, arg6 };
if (CHIP_NO_ERROR != LogEvent(event, commandPath.mEndpointId, eventOptions, responseData.value))
{
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_FAILURE);
return true;
}
commandObj->AddResponseData(commandPath, responseData);
return true;
}

bool emberAfTestClusterClusterTestListInt8UArgumentRequestCallback(
app::CommandHandler * commandObj, app::ConcreteCommandPath const & commandPath,
CommandHandler * commandObj, ConcreteCommandPath const & commandPath,
Commands::TestListInt8UArgumentRequest::DecodableType const & commandData)
{
bool shouldReturnTrue = true;
Expand Down
29 changes: 29 additions & 0 deletions src/app/server/Server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <app/server/Server.h>

#include <app/EventManagement.h>
#include <app/InteractionModelEngine.h>
#include <app/server/Dnssd.h>
#include <app/server/EchoHandler.h>
Expand Down Expand Up @@ -70,6 +71,16 @@ namespace chip {

Server Server::sServer;

#define CHIP_NUM_EVENT_LOGGING_BUFFERS 3
static uint8_t sCritEventBuffer[CHIP_DEVICE_CONFIG_EVENT_LOGGING_CRIT_BUFFER_SIZE];
static uint8_t sInfoEventBuffer[CHIP_DEVICE_CONFIG_EVENT_LOGGING_INFO_BUFFER_SIZE];
static uint8_t sDebugEventBuffer[CHIP_DEVICE_CONFIG_EVENT_LOGGING_DEBUG_BUFFER_SIZE];
static ::chip::PersistedCounter sCritEventIdCounter;
static ::chip::PersistedCounter sInfoEventIdCounter;
static ::chip::PersistedCounter sDebugEventIdCounter;

static ::chip::app::CircularEventBuffer sLoggingBuffer[CHIP_NUM_EVENT_LOGGING_BUFFERS];

CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint16_t unsecureServicePort)
{
mSecuredServicePort = secureServicePort;
Expand Down Expand Up @@ -123,6 +134,24 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
err = chip::app::InteractionModelEngine::GetInstance()->Init(&mExchangeMgr, nullptr);
SuccessOrExit(err);

// Initialize event logging subsystem
{
::chip::Platform::PersistedStorage::Key critEventIdCounterStorageKey = CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY;
::chip::Platform::PersistedStorage::Key infoEventIdCounterStorageKey = CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY;
::chip::Platform::PersistedStorage::Key debugEventIdCounterStorageKey = CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_DEBUG_EIDC_KEY;

::chip::app::LogStorageResources logStorageResources[] = {
{ &sCritEventBuffer[0], sizeof(sCritEventBuffer), &critEventIdCounterStorageKey,
CHIP_DEVICE_CONFIG_EVENT_ID_COUNTER_EPOCH, &sCritEventIdCounter, ::chip::app::PriorityLevel::Critical },
{ &sInfoEventBuffer[0], sizeof(sInfoEventBuffer), &infoEventIdCounterStorageKey,
CHIP_DEVICE_CONFIG_EVENT_ID_COUNTER_EPOCH, &sInfoEventIdCounter, ::chip::app::PriorityLevel::Info },
{ &sDebugEventBuffer[0], sizeof(sDebugEventBuffer), &debugEventIdCounterStorageKey,
CHIP_DEVICE_CONFIG_EVENT_ID_COUNTER_EPOCH, &sDebugEventIdCounter, ::chip::app::PriorityLevel::Debug }
};

chip::app::EventManagement::GetInstance().Init(&mExchangeMgr, CHIP_NUM_EVENT_LOGGING_BUFFERS, &sLoggingBuffer[0],
&logStorageResources[0]);
}
#if defined(CHIP_APP_USE_ECHO)
err = InitEchoHandler(&gExchangeMgr);
SuccessOrExit(err);
Expand Down
20 changes: 20 additions & 0 deletions src/app/zap-templates/zcl/data-model/chip/test-cluster.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,19 @@ limitations under the License.
array="true" optional="true"/>
</command>

<command source="client" code="0x11" name="TestEmitTestEventRequest"
optional="false" response="TestEmitTestEventResponse">
<description>
Command that takes identical arguments to the fields of the TestEvent and logs the TestEvent to the buffer. Command returns an event ID as the response.
</description>
<arg name="arg1" type="INT8U"/>
<arg name="arg2" type="SimpleEnum"/>
<arg name="arg3" type="BOOLEAN"/>
<arg name="arg4" type="SimpleStruct"/>
<arg name="arg5" type="SimpleStruct" array="true"/>
<arg name="arg6" type="SimpleEnum" array="true"/>
</command>

<command source="server" code="0x00" name="TestSpecificResponse" optional="false" disableDefaultResponse="true">
<description>
Simple response for TestWithResponse with a simple return value
Expand Down Expand Up @@ -437,6 +450,13 @@ limitations under the License.
<arg name="value" type="BOOLEAN"/>
</command>

<command source="server" code="0x09" name="TestEmitTestEventResponse" optional="false" disableDefaultResponse="true">
<description>
Delivers the ID of an event in response to test command emitting events.
</description>
<arg name="value" type="INT64U"/>
</command>

<event code="0x0001" name="TestEvent" priority="info" side="server">
<description>Example test event</description>
<field id="1" name="arg1" type="INT8U"/>
Expand Down
47 changes: 47 additions & 0 deletions src/controller/python/chip/clusters/Objects.py
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions src/darwin/Framework/CHIP/zap-generated/CHIPCommandPayloadsObjc.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/darwin/Framework/CHIP/zap-generated/CHIPCommandPayloadsObjc.mm
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 0 additions & 10 deletions src/include/platform/CHIPDeviceConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -896,16 +896,6 @@
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY "crit-eidc"
#endif

/**
* @def CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY
*
* @brief
* The production event id counter (eidc) persisted storage key.
*/
#ifndef CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY "prod-eidc"
#endif

/**
* @def CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY
*
Expand Down
5 changes: 0 additions & 5 deletions src/platform/Ameba/CHIPDevicePlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@

#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 0

#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY 2
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY 3
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY 4
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_DEBUG_EIDC_KEY 5

// ========== Platform-specific Configuration =========

// These are configuration options that are unique to the platform.
Expand Down
5 changes: 0 additions & 5 deletions src/platform/Darwin/CHIPDevicePlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@

#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 0

#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY 2
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY 3
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY 4
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_DEBUG_EIDC_KEY 5

#define CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DISCOVERY 1

// ========== Platform-specific Configuration =========
Expand Down
5 changes: 0 additions & 5 deletions src/platform/EFR32/CHIPDevicePlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,6 @@

#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 0

#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY 2
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY 3
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY 4
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_DEBUG_EIDC_KEY 5

// ========== Platform-specific Configuration =========

// These are configuration options that are unique to the EFR32 platform.
Expand Down
5 changes: 0 additions & 5 deletions src/platform/Linux/CHIPDevicePlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@

#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 0

#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY 2
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY 3
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY 4
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_DEBUG_EIDC_KEY 5

#define CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DISCOVERY 1

// ========== Platform-specific Configuration =========
Expand Down
5 changes: 0 additions & 5 deletions src/platform/Tizen/CHIPDevicePlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@

#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 0

#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY 2
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY 3
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY 4
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_DEBUG_EIDC_KEY 5

// ========== Platform-specific Configuration =========

// These are configuration options that are unique to Tizen platforms.
Expand Down
5 changes: 0 additions & 5 deletions src/platform/android/CHIPDevicePlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@

#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 0

#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY 2
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY 3
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY 4
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_DEBUG_EIDC_KEY 5

#define CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS 1

// ========== Platform-specific Configuration =========
Expand Down
5 changes: 0 additions & 5 deletions src/platform/mbed/CHIPDevicePlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@

#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 0

#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY 2
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY 3
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY 4
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_DEBUG_EIDC_KEY 5

#define CHIP_DEVICE_CONFIG_ENABLE_DNSSD 1

#ifndef CHIP_DEVICE_CONFIG_CHIP_TASK_STACK_SIZE
Expand Down
5 changes: 0 additions & 5 deletions src/platform/nrfconnect/CHIPDevicePlatformConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@

#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 0

#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_CRIT_EIDC_KEY 2
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_PROD_EIDC_KEY 3
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_INFO_EIDC_KEY 4
#define CHIP_DEVICE_CONFIG_PERSISTED_STORAGE_DEBUG_EIDC_KEY 5

// ========== Platform-specific Configuration =========

// These are configuration options that are unique to Zephyr platforms.
Expand Down
Loading