Replace and remove weakly-typed System::Clock code (#11238)

* WIP

* Replace and remove weakly-typed System::Clock code

#### Problem

Previous PRs added strong types to `System::Clock` and converted many
uses, but a few remain.

#### Change overview

Convert the remaining uses of the weak types (`MonotonicMilliseconds`
and `MonotonicMicroseconds`), and remove them from `System::Clock`.

Fixes #10062 Some operations on System::Clock types are not safe

There are some remaining uses of `Clock`-derived values as plain
integers; these can be found for later cleanup by the use of `.count()`.

#### Testing

CI; no changes to functionality.
Converted `TestPlatformTime`.

* review

* initialization consistency

* restyle

examples/lock-app/esp32/main/AppTask.cpp

@@ -114,7 +114,7 @@ CHIP_ERROR AppTask::Init()
 void AppTask::AppTaskMain(void * pvParameter)
 {
     AppEvent event;
-    Clock::MonotonicMicroseconds mLastChangeTimeUS = 0;
+    Clock::Timestamp lastChangeTime = Clock::Zero;
 
     CHIP_ERROR err = sAppTask.Init();
     if (err != CHIP_NO_ERROR)
@@ -170,12 +170,12 @@ void AppTask::AppTaskMain(void * pvParameter)
         sStatusLED.Animate();
         sLockLED.Animate();
 
-        Clock::MonotonicMicroseconds nowUS            = SystemClock().GetMonotonicMicroseconds();
-        Clock::MonotonicMicroseconds nextChangeTimeUS = mLastChangeTimeUS + (5 * 1000 * 1000UL);
+        Clock::Timestamp now            = SystemClock().GetMonotonicTimestamp();
+        Clock::Timestamp nextChangeTime = lastChangeTime + Clock::Seconds16(5);
 
-        if (nextChangeTimeUS < nowUS)
+        if (nextChangeTime < now)
         {
-            mLastChangeTimeUS = nowUS;
+            lastChangeTime = now;
         }
         if (lockButton.Poll())
         {

examples/lock-app/esp32/main/LEDWidget.cpp

@@ -22,7 +22,7 @@
 
 void LEDWidget::Init(gpio_num_t gpioNum)
 {
-    mLastChangeTimeUS = 0;
+    mLastChangeTimeMS = 0;
     mBlinkOnTimeMS    = 0;
     mBlinkOffTimeMS   = 0;
     mGPIONum          = gpioNum;
@@ -61,14 +61,14 @@ void LEDWidget::Animate()
 {
     if (mBlinkOnTimeMS != 0 && mBlinkOffTimeMS != 0)
     {
-        chip::System::Clock::MonotonicMicroseconds nowUS            = chip::System::SystemClock().GetMonotonicMicroseconds();
-        chip::System::Clock::MonotonicMicroseconds stateDurUS       = ((mState) ? mBlinkOnTimeMS : mBlinkOffTimeMS) * 1000LL;
-        chip::System::Clock::MonotonicMicroseconds nextChangeTimeUS = mLastChangeTimeUS + stateDurUS;
+        uint64_t nowMS            = chip::System::SystemClock().GetMonotonicMilliseconds64().count();
+        uint64_t stateDurMS       = ((mState) ? mBlinkOnTimeMS : mBlinkOffTimeMS);
+        uint64_t nextChangeTimeMS = mLastChangeTimeMS + stateDurMS;
 
-        if (nextChangeTimeUS < nowUS)
+        if (nextChangeTimeMS < nowMS)
         {
             DoSet(!mState);
-            mLastChangeTimeUS = nowUS;
+            mLastChangeTimeMS = nowMS;
         }
     }
 }

examples/lock-app/esp32/main/include/LEDWidget.h

@@ -33,7 +33,7 @@ class LEDWidget
     void Animate();
 
 private:
-    uint64_t mLastChangeTimeUS;
+    uint64_t mLastChangeTimeMS;
     uint32_t mBlinkOnTimeMS;
     uint32_t mBlinkOffTimeMS;
     gpio_num_t mGPIONum;

examples/platform/efr32/LEDWidget.cpp

@@ -32,7 +32,7 @@ void LEDWidget::InitGpio(void)
 
 void LEDWidget::Init(const sl_led_t * led)
 {
-    mLastChangeTimeUS = 0;
+    mLastChangeTimeMS = 0;
     mBlinkOnTimeMS    = 0;
     mBlinkOffTimeMS   = 0;
     mLed              = led;
@@ -50,7 +50,7 @@ void LEDWidget::Invert(void)
 
 void LEDWidget::Set(bool state)
 {
-    mLastChangeTimeUS = mBlinkOnTimeMS = mBlinkOffTimeMS = 0;
+    mLastChangeTimeMS = mBlinkOnTimeMS = mBlinkOffTimeMS = 0;
     if (mLed)
     {
         state ? sl_led_turn_on(mLed) : sl_led_turn_off(mLed);
@@ -73,14 +73,14 @@ void LEDWidget::Animate()
 {
     if (mBlinkOnTimeMS != 0 && mBlinkOffTimeMS != 0)
     {
-        Clock::MonotonicMicroseconds nowUS            = chip::System::SystemClock().GetMonotonicMicroseconds();
-        Clock::MonotonicMicroseconds stateDurUS       = ((sl_led_get_state(mLed)) ? mBlinkOnTimeMS : mBlinkOffTimeMS) * 1000LL;
-        Clock::MonotonicMicroseconds nextChangeTimeUS = mLastChangeTimeUS + stateDurUS;
+        uint64_t nowMS            = chip::System::SystemClock().GetMonotonicMilliseconds64().count();
+        uint64_t stateDurMS       = sl_led_get_state(mLed) ? mBlinkOnTimeMS : mBlinkOffTimeMS;
+        uint64_t nextChangeTimeMS = mLastChangeTimeMS + stateDurMS;
 
-        if (nextChangeTimeUS < nowUS)
+        if (nextChangeTimeMS < nowMS)
         {
             Invert();
-            mLastChangeTimeUS = nowUS;
+            mLastChangeTimeMS = nowMS;
         }
     }
 }

examples/platform/efr32/LEDWidget.h

@@ -34,7 +34,7 @@ class LEDWidget
     void Animate();
 
 private:
-    uint64_t mLastChangeTimeUS;
+    uint64_t mLastChangeTimeMS;
     uint32_t mBlinkOnTimeMS;
     uint32_t mBlinkOffTimeMS;
     const sl_led_t * mLed;

examples/platform/mbed/util/LEDWidget.cpp

@@ -57,9 +57,9 @@ void LEDWidget::Animate()
 {
     if (mBlinkOnTimeMS != 0 && mBlinkOffTimeMS != 0)
     {
-        chip::System::Clock::MonotonicMilliseconds nowMS            = chip::System::SystemClock().GetMonotonicMilliseconds();
-        chip::System::Clock::MonotonicMilliseconds stateDurMS       = (mLED == LED_ACTIVE_STATE) ? mBlinkOnTimeMS : mBlinkOffTimeMS;
-        chip::System::Clock::MonotonicMilliseconds nextChangeTimeMS = mLastChangeTimeMS + stateDurMS;
+        uint64_t nowMS            = chip::System::SystemClock().GetMonotonicMilliseconds64().count();
+        uint64_t stateDurMS       = (mLED == LED_ACTIVE_STATE) ? mBlinkOnTimeMS : mBlinkOffTimeMS;
+        uint64_t nextChangeTimeMS = mLastChangeTimeMS + stateDurMS;
 
         if (nextChangeTimeMS < nowMS)
         {

examples/platform/nxp/k32w/k32w0/util/LEDWidget.cpp

@@ -23,7 +23,7 @@
 
 void LEDWidget::Init(LED_t led)
 {
-    mLastChangeTimeUS = 0;
+    mLastChangeTimeMS = 0;
     mBlinkOnTimeMS    = 0;
     mBlinkOffTimeMS   = 0;
     mGPIONum          = led;
@@ -39,7 +39,7 @@ void LEDWidget::Invert(void)
 
 void LEDWidget::Set(bool state)
 {
-    mLastChangeTimeUS = mBlinkOnTimeMS = mBlinkOffTimeMS = 0;
+    mLastChangeTimeMS = mBlinkOnTimeMS = mBlinkOffTimeMS = 0;
     DoSet(state);
 }
 
@@ -59,14 +59,14 @@ void LEDWidget::Animate()
 {
     if (mBlinkOnTimeMS != 0 && mBlinkOffTimeMS != 0)
     {
-        chip::System::Clock::MonotonicMicroseconds nowUS            = chip::System::SystemClock().GetMonotonicMicroseconds();
-        chip::System::Clock::MonotonicMicroseconds stateDurUS       = ((mState) ? mBlinkOnTimeMS : mBlinkOffTimeMS) * 1000LL;
-        chip::System::Clock::MonotonicMicroseconds nextChangeTimeUS = mLastChangeTimeUS + stateDurUS;
+        uint64_t nowMS            = chip::System::SystemClock().GetMonotonicMilliseconds64().count();
+        uint64_t stateDurMS       = mState ? mBlinkOnTimeMS : mBlinkOffTimeMS;
+        uint64_t nextChangeTimeMS = mLastChangeTimeMS + stateDurMS;
 
-        if (nextChangeTimeUS < nowUS)
+        if (nextChangeTimeMS < nowMS)
         {
             DoSet(!mState);
-            mLastChangeTimeUS = nowUS;
+            mLastChangeTimeMS = nowMS;
         }
     }
 }

examples/platform/nxp/k32w/k32w0/util/include/LEDWidget.h

@@ -31,7 +31,7 @@ class LEDWidget
     void Animate();
 
 private:
-    uint64_t mLastChangeTimeUS;
+    uint64_t mLastChangeTimeMS;
     uint32_t mBlinkOnTimeMS;
     uint32_t mBlinkOffTimeMS;
     LED_t mGPIONum;

examples/platform/p6/LEDWidget.cpp

@@ -25,7 +25,7 @@
 
 void LEDWidget::Init(int ledNum)
 {
-    mLastChangeTimeUS = 0;
+    mLastChangeTimeMS = 0;
     mBlinkOnTimeMS    = 0;
     mBlinkOffTimeMS   = 0;
     mLedNum           = ledNum;
@@ -41,7 +41,7 @@ void LEDWidget::Invert(void)
 
 void LEDWidget::Set(bool state)
 {
-    mLastChangeTimeUS = mBlinkOnTimeMS = mBlinkOffTimeMS = 0;
+    mLastChangeTimeMS = mBlinkOnTimeMS = mBlinkOffTimeMS = 0;
     DoSet(state);
 }
 
@@ -61,14 +61,14 @@ void LEDWidget::Animate()
 {
     if (mBlinkOnTimeMS != 0 && mBlinkOffTimeMS != 0)
     {
-        chip::System::Clock::MonotonicMicroseconds nowUS            = chip::System::SystemClock().GetMonotonicMicroseconds();
-        chip::System::Clock::MonotonicMicroseconds stateDurUS       = ((mState) ? mBlinkOnTimeMS : mBlinkOffTimeMS) * 1000LL;
-        chip::System::Clock::MonotonicMicroseconds nextChangeTimeUS = mLastChangeTimeUS + stateDurUS;
+        uint64_t nowMS            = chip::System::SystemClock().GetMonotonicMilliseconds64().count();
+        uint64_t stateDurMS       = mState ? mBlinkOnTimeMS : mBlinkOffTimeMS;
+        uint64_t nextChangeTimeMS = mLastChangeTimeMS + stateDurMS;
 
-        if (nextChangeTimeUS < nowUS)
+        if (nextChangeTimeMS < nowMS)
         {
             DoSet(!mState);
-            mLastChangeTimeUS = nowUS;
+            mLastChangeTimeMS = nowMS;
         }
     }
 }

examples/platform/p6/LEDWidget.h

@@ -34,7 +34,7 @@ class LEDWidget
     void Animate();
 
 private:
-    uint64_t mLastChangeTimeUS = 0;
+    uint64_t mLastChangeTimeMS = 0;
     uint32_t mBlinkOnTimeMS    = 0;
     uint32_t mBlinkOffTimeMS   = 0;
     int mLedNum                = 0;

examples/shell/shell_common/cmd_ping.cpp

@@ -50,7 +50,7 @@ class PingArguments
     {
         mMaxEchoCount       = 3;
         mEchoInterval       = 1000;
-        mLastEchoTime       = 0;
+        mLastEchoTime       = System::Clock::Zero;
         mEchoCount          = 0;
         mEchoRespCount      = 0;
         mPayloadSize        = 32;
@@ -62,8 +62,8 @@ class PingArguments
         mEchoPort = CHIP_PORT;
     }
 
-    uint64_t GetLastEchoTime() const { return mLastEchoTime; }
-    void SetLastEchoTime(uint64_t value) { mLastEchoTime = value; }
+    System::Clock::Timestamp GetLastEchoTime() const { return mLastEchoTime; }
+    void SetLastEchoTime(System::Clock::Timestamp value) { mLastEchoTime = value; }
 
     uint64_t GetEchoCount() const { return mEchoCount; }
     void SetEchoCount(uint64_t value) { mEchoCount = value; }
@@ -98,7 +98,7 @@ class PingArguments
 
 private:
     // The last time a echo request was attempted to be sent.
-    uint64_t mLastEchoTime;
+    System::Clock::Timestamp mLastEchoTime;
 
     // Count of the number of echo requests sent.
     uint64_t mEchoCount;
@@ -210,7 +210,7 @@ CHIP_ERROR SendEchoRequest(streamer_t * stream)
         sendFlags.Set(Messaging::SendMessageFlags::kNoAutoRequestAck);
     }
 
-    gPingArguments.SetLastEchoTime(System::SystemClock().GetMonotonicMilliseconds());
+    gPingArguments.SetLastEchoTime(System::SystemClock().GetMonotonicTimestamp());
     SuccessOrExit(chip::DeviceLayer::SystemLayer().StartTimer(chip::System::Clock::Milliseconds32(gPingArguments.GetEchoInterval()),
                                                               EchoTimerHandler, NULL));
 
@@ -256,7 +256,7 @@ CHIP_ERROR EstablishSecureSession(streamer_t * stream, const Transport::PeerAddr
     if (err != CHIP_NO_ERROR)
     {
         streamer_printf(stream, "Establish secure session failed, err: %s\n", ErrorStr(err));
-        gPingArguments.SetLastEchoTime(System::SystemClock().GetMonotonicMilliseconds());
+        gPingArguments.SetLastEchoTime(System::SystemClock().GetMonotonicTimestamp());
     }
     else
     {
@@ -268,17 +268,17 @@ CHIP_ERROR EstablishSecureSession(streamer_t * stream, const Transport::PeerAddr
 
 void HandleEchoResponseReceived(Messaging::ExchangeContext * ec, System::PacketBufferHandle && payload)
 {
-    uint64_t respTime    = System::SystemClock().GetMonotonicMilliseconds();
-    uint64_t transitTime = respTime - gPingArguments.GetLastEchoTime();
-    streamer_t * sout    = streamer_get();
+    System::Clock::Timestamp respTime         = System::SystemClock().GetMonotonicTimestamp();
+    System::Clock::Milliseconds64 transitTime = respTime - gPingArguments.GetLastEchoTime();
+    streamer_t * sout                         = streamer_get();
 
     gPingArguments.SetWaitingForEchoResp(false);
     gPingArguments.IncrementEchoRespCount();
 
-    streamer_printf(sout, "Echo Response: %" PRIu64 "/%" PRIu64 "(%.2f%%) len=%u time=%.3fms\n", gPingArguments.GetEchoRespCount(),
+    streamer_printf(sout, "Echo Response: %" PRIu64 "/%" PRIu64 "(%.2f%%) len=%u time=%.3fs\n", gPingArguments.GetEchoRespCount(),
                     gPingArguments.GetEchoCount(),
                     static_cast<double>(gPingArguments.GetEchoRespCount()) * 100 / gPingArguments.GetEchoCount(),
-                    payload->DataLength(), static_cast<double>(transitTime) / 1000);
+                    payload->DataLength(), static_cast<double>(transitTime.count()) / 1000);
 }
 
 void StartPinging(streamer_t * stream, char * destination)

examples/shell/shell_common/cmd_send.cpp

@@ -47,7 +47,7 @@ class SendArguments
     {
         mProtocolId   = 0x0002;
         mMessageType  = 1;
-        mLastSendTime = 0;
+        mLastSendTime = System::Clock::Zero;
         mPayloadSize  = 32;
 #if INET_CONFIG_ENABLE_TCP_ENDPOINT
         mUsingTCP = false;
@@ -56,8 +56,8 @@ class SendArguments
         mPort     = CHIP_PORT;
     }
 
-    uint64_t GetLastSendTime() const { return mLastSendTime; }
-    void SetLastSendTime(uint64_t value) { mLastSendTime = value; }
+    System::Clock::Timestamp GetLastSendTime() const { return mLastSendTime; }
+    void SetLastSendTime(System::Clock::Timestamp value) { mLastSendTime = value; }
 
     uint16_t GetProtocolId() const { return mProtocolId; }
     void SetProtocolId(uint16_t value) { mProtocolId = value; }
@@ -81,7 +81,7 @@ class SendArguments
 
 private:
     // The last time a CHIP message was attempted to be sent.
-    uint64_t mLastSendTime;
+    System::Clock::Timestamp mLastSendTime;
 
     uint32_t mPayloadSize;
     uint16_t mProtocolId;
@@ -101,12 +101,12 @@ class MockAppDelegate : public Messaging::ExchangeDelegate
     CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader,
                                  System::PacketBufferHandle && buffer) override
     {
-        uint64_t respTime    = System::SystemClock().GetMonotonicMilliseconds();
-        uint64_t transitTime = respTime - gSendArguments.GetLastSendTime();
-        streamer_t * sout    = streamer_get();
+        System::Clock::Timestamp respTime         = System::SystemClock().GetMonotonicTimestamp();
+        System::Clock::Milliseconds64 transitTime = respTime - gSendArguments.GetLastSendTime();
+        streamer_t * sout                         = streamer_get();
 
-        streamer_printf(sout, "Response received: len=%u time=%.3fms\n", buffer->DataLength(),
-                        static_cast<double>(transitTime) / 1000);
+        streamer_printf(sout, "Response received: len=%u time=%.3fs\n", buffer->DataLength(),
+                        static_cast<double>(transitTime.count()) / 1000);
 
         return CHIP_NO_ERROR;
     }
@@ -148,7 +148,7 @@ CHIP_ERROR SendMessage(streamer_t * stream)
     ec->SetResponseTimeout(kResponseTimeOut);
     sendFlags.Set(Messaging::SendMessageFlags::kExpectResponse);
 
-    gSendArguments.SetLastSendTime(System::SystemClock().GetMonotonicMilliseconds());
+    gSendArguments.SetLastSendTime(System::SystemClock().GetMonotonicTimestamp());
 
     streamer_printf(stream, "\nSend CHIP message with payload size: %d bytes to Node: %" PRIu64 "\n", payloadSize,
                     kTestDeviceNodeId);
@@ -187,7 +187,7 @@ CHIP_ERROR EstablishSecureSession(streamer_t * stream, Transport::PeerAddress &
     if (err != CHIP_NO_ERROR)
     {
         streamer_printf(stream, "Establish secure session failed, err: %s\n", ErrorStr(err));
-        gSendArguments.SetLastSendTime(System::SystemClock().GetMonotonicMilliseconds());
+        gSendArguments.SetLastSendTime(System::SystemClock().GetMonotonicTimestamp());
     }
     else
     {

src/app/EventLoggingTypes.h

@@ -117,14 +117,15 @@ struct Timestamp
     Timestamp() {}
     Timestamp(Type aType) : mType(aType) { mValue = 0; }
     Timestamp(Type aType, uint64_t aValue) : mType(aType), mValue(aValue) {}
+    Timestamp(System::Clock::Timestamp aValue) : mType(Type::kSystem), mValue(aValue.count()) {}
     static Timestamp UTC(uint64_t aValue)
     {
         Timestamp timestamp(Type::kUTC, aValue);
         return timestamp;
     }
-    static Timestamp System(uint64_t aValue)
+    static Timestamp System(System::Clock::Timestamp aValue)
     {
-        Timestamp timestamp(Type::kSystem, aValue);
+        Timestamp timestamp(Type::kSystem, aValue.count());
         return timestamp;
     }
 

src/app/EventManagement.cpp

@@ -87,7 +87,7 @@ struct EventEnvelopeContext
 
     uint16_t mFieldsToRead = 0;
     /* PriorityLevel and DeltaSystemTimestamp are there if that is not first event when putting events in report*/
-    Timestamp mDeltaSystemTime = Timestamp::System(0);
+    Timestamp mDeltaSystemTime = Timestamp::System(System::Clock::Zero);
     Timestamp mDeltaUtc        = Timestamp::UTC(0);
     PriorityLevel mPriority    = PriorityLevel::First;
     NodeId mNodeId             = 0;
@@ -496,7 +496,7 @@ CHIP_ERROR EventManagement::LogEventPrivate(EventLoggingDelegate * apDelegate, E
     CircularEventBuffer * buffer   = nullptr;
     EventLoadOutContext ctxt       = EventLoadOutContext(writer, aEventOptions.mpEventSchema->mPriority,
                                                    GetPriorityBuffer(aEventOptions.mpEventSchema->mPriority)->GetLastEventNumber());
-    Timestamp timestamp(Timestamp::Type::kSystem, System::SystemClock().GetMonotonicMilliseconds());
+    Timestamp timestamp(System::SystemClock().GetMonotonicTimestamp());
     EventOptions opts = EventOptions(timestamp);
     // Start the event container (anonymous structure) in the circular buffer
     writer.Init(*mpEventBuffer);
@@ -854,8 +854,8 @@ void CircularEventBuffer::Init(uint8_t * apBuffer, uint32_t aBufferLength, Circu
     mPriority                  = aPriorityLevel;
     mFirstEventNumber          = 1;
     mLastEventNumber           = 0;
-    mFirstEventSystemTimestamp = Timestamp::System(0);
-    mLastEventSystemTimestamp  = Timestamp::System(0);
+    mFirstEventSystemTimestamp = Timestamp::System(System::Clock::Zero);
+    mLastEventSystemTimestamp  = Timestamp::System(System::Clock::Zero);
     mpEventNumberCounter       = nullptr;
 }