Skip to content

Commit

Permalink
Fix timestamps in Diagnostic Logs cluster to follow the spec.
Browse files Browse the repository at this point in the history
There were a few issues here:

1. We were using a monotonic timestamp, but setting the UTCTimeStamp in the
response payload.

2. We were using a millisecond timestamp, whereas the spec has microsecond ones.

Switch to using "time since server init" for the timestamp, put it in the
TimeSinceBoot field, and make sure it has the right units.

Also fixes the timestamps to actually be optional, per spec.

Fixes #24265
  • Loading branch information
bzbarsky-apple committed Feb 2, 2023
1 parent 94c7f68 commit b7b1af2
Show file tree
Hide file tree
Showing 16 changed files with 99 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,8 @@ server cluster DiagnosticLogs = 50 {
response struct RetrieveLogsResponse = 1 {
LogsStatus status = 0;
OCTET_STRING logContent = 1;
epoch_s UTCTimeStamp = 2;
INT32U timeSinceBoot = 3;
optional epoch_us UTCTimeStamp = 2;
optional systime_us timeSinceBoot = 3;
}

command RetrieveLogsRequest(RetrieveLogsRequestRequest): RetrieveLogsResponse = 0;
Expand Down
30 changes: 18 additions & 12 deletions src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,20 @@
#include <app/CommandHandlerInterface.h>
#include <app/ConcreteCommandPath.h>
#include <app/clusters/diagnostic-logs-server/diagnostic-logs-server.h>
#include <app/server/Server.h>
#include <app/util/af.h>
#include <lib/support/BytesCircularBuffer.h>
#include <lib/support/ScopedBuffer.h>

#include <array>

// We store our times as millisecond times, to save space.
using TimeInBufferType = chip::System::Clock::Milliseconds32::rep;

CHIP_ERROR DiagnosticLogsCommandHandler::PushLog(const chip::ByteSpan & payload)
{
chip::System::Clock::Milliseconds32 now = chip::System::SystemClock().GetMonotonicTimestamp();
uint32_t timeMs = now.count();
auto now = std::chrono::duration_cast<chip::System::Clock::Milliseconds32>(chip::Server::GetInstance().TimeSinceInit());
TimeInBufferType timeMs = now.count();
chip::ByteSpan payloadTime(reinterpret_cast<uint8_t *>(&timeMs), sizeof(timeMs));
return mBuffer.Push(payloadTime, payload);
}
Expand Down Expand Up @@ -56,27 +61,28 @@ void DiagnosticLogsCommandHandler::InvokeCommand(HandlerContext & handlerContext
}

size_t logSize = mBuffer.GetFrontSize();
chip::System::Clock::Milliseconds32::rep timeMs;
VerifyOrDie(logSize > sizeof(timeMs));
TimeInBufferType timeFromBuffer;
VerifyOrDie(logSize > sizeof(timeFromBuffer));

std::unique_ptr<uint8_t, decltype(&chip::Platform::MemoryFree)> buf(
reinterpret_cast<uint8_t *>(chip::Platform::MemoryAlloc(logSize)), &chip::Platform::MemoryFree);
if (!buf)
chip::Platform::ScopedMemoryBuffer<uint8_t> buf;
if (!buf.Calloc(logSize))
{
response.status = chip::app::Clusters::DiagnosticLogs::LogsStatus::kBusy;
handlerContext.mCommandHandler.AddResponse(handlerContext.mRequestPath, response);
break;
}

// The entry is | time (4 bytes) | content (var size) |
chip::MutableByteSpan entry(buf.get(), logSize);
chip::MutableByteSpan entry(buf.Get(), logSize);
CHIP_ERROR err = mBuffer.ReadFront(entry);
VerifyOrDie(err == CHIP_NO_ERROR);
timeMs = *reinterpret_cast<decltype(timeMs) *>(buf.get());
memcpy(&timeFromBuffer, buf.Get(), sizeof(timeFromBuffer));

auto timestamp = chip::System::Clock::Milliseconds32(timeFromBuffer);

response.status = chip::app::Clusters::DiagnosticLogs::LogsStatus::kSuccess;
response.logContent = chip::ByteSpan(buf.get() + sizeof(timeMs), logSize - sizeof(timeMs));
response.UTCTimeStamp = timeMs;
response.status = chip::app::Clusters::DiagnosticLogs::LogsStatus::kSuccess;
response.logContent = chip::ByteSpan(buf.Get() + sizeof(timeFromBuffer), logSize - sizeof(timeFromBuffer));
response.timeSinceBoot.SetValue(chip::System::Clock::Microseconds64(timestamp).count());
handlerContext.mCommandHandler.AddResponse(handlerContext.mRequestPath, response);
}
break;
Expand Down
2 changes: 2 additions & 0 deletions src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
{
ChipLogProgress(AppServer, "Server initializing...");

mInitTimestamp = System::SystemClock().GetMonotonicMicroseconds64();

CASESessionManagerConfig caseSessionManagerConfig;
DeviceLayer::DeviceInfoProvider * deviceInfoprovider = nullptr;

Expand Down
5 changes: 5 additions & 0 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include <protocols/secure_channel/SimpleSessionResumptionStorage.h>
#endif
#include <protocols/user_directed_commissioning/UserDirectedCommissioning.h>
#include <system/SystemClock.h>
#include <transport/SessionManager.h>
#include <transport/TransportMgr.h>
#include <transport/TransportMgrBase.h>
Expand Down Expand Up @@ -369,6 +370,8 @@ class Server

void ScheduleFactoryReset();

System::Clock::Microseconds64 TimeSinceInit() { return System::SystemClock().GetMonotonicMicroseconds64() - mInitTimestamp; }

static Server & GetInstance() { return sServer; }

private:
Expand Down Expand Up @@ -566,6 +569,8 @@ class Server
uint16_t mOperationalServicePort;
uint16_t mUserDirectedCommissioningPort;
Inet::InterfaceId mInterfaceId;

System::Clock::Microseconds64 mInitTimestamp;
};

} // namespace chip
1 change: 1 addition & 0 deletions src/app/zap-templates/common/override.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ function atomicType(arg)
case 'percent100ths':
return 'chip::Percent100ths';
case 'epoch_us':
case 'systime_us':
return 'uint64_t';
case 'epoch_s':
case 'utc':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ limitations under the License.
<description>Response to the RetrieveLogsRequest</description>
<arg name="Status" type="LogsStatus"/>
<arg name="LogContent" type="OCTET_STRING"/>
<arg name="UTCTimeStamp" type="epoch_s"/>
<arg name="TimeSinceBoot" type="INT32U"/>
<arg name="UTCTimeStamp" type="epoch_us" optional="true"/>
<arg name="TimeSinceBoot" type="systime_us" optional="true"/>
</command>
</cluster>
</configurator>
4 changes: 2 additions & 2 deletions src/controller/data_model/controller-clusters.matter
Original file line number Diff line number Diff line change
Expand Up @@ -1286,8 +1286,8 @@ client cluster DiagnosticLogs = 50 {
response struct RetrieveLogsResponse = 1 {
LogsStatus status = 0;
OCTET_STRING logContent = 1;
epoch_s UTCTimeStamp = 2;
INT32U timeSinceBoot = 3;
optional epoch_us UTCTimeStamp = 2;
optional systime_us timeSinceBoot = 3;
}

command RetrieveLogsRequest(RetrieveLogsRequestRequest): RetrieveLogsResponse = 0;
Expand Down
38 changes: 29 additions & 9 deletions src/controller/java/zap-generated/CHIPInvokeCallbacks.cpp

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

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

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

8 changes: 4 additions & 4 deletions src/controller/python/chip/clusters/Objects.py

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

12 changes: 10 additions & 2 deletions src/darwin/Framework/CHIP/zap-generated/MTRCallbackBridge.mm

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

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

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

4 changes: 2 additions & 2 deletions zzz_generated/app-common/app-common/zap-generated/callback.h

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

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

0 comments on commit b7b1af2

Please sign in to comment.