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

Fix timestamps in Diagnostic Logs cluster to follow the spec. #24830

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
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,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.
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -108,6 +108,8 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
ChipLogProgress(AppServer, "Server initializing...");
assertChipStackLockedByCurrentThread();

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

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

Expand Down
8 changes: 8 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,11 @@ class Server

void ScheduleFactoryReset();

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

static Server & GetInstance() { return sServer; }

private:
Expand Down Expand Up @@ -566,6 +572,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 @@ -1317,8 +1317,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.