Skip to content

Commit

Permalink
Use safer System::Clock types in chip-tool (#10854)
Browse files Browse the repository at this point in the history
#### Problem

Code uses plain integers to represent time values and relies on
users to get the unit scale correct.

Part of #10062 _Some operations on System::Clock types are not safe_

#### Change overview

Convert `examples/chip-tool` to use the safer `Clock` types.

#### Testing

CI; no change to functionality intended.
  • Loading branch information
kpschoedel authored and pull[bot] committed Oct 26, 2021
1 parent 067dd7f commit 1092138
Show file tree
Hide file tree
Showing 11 changed files with 18 additions and 18 deletions.
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/clusters/ModelCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class ModelCommand : public CHIPCommand

/////////// CHIPCommand Interface /////////
CHIP_ERROR RunCommand() override;
uint16_t GetWaitDurationInSeconds() const override { return 10; }
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(10); }

virtual CHIP_ERROR SendCommand(ChipDevice * device, uint8_t endPointId) = 0;

Expand Down
9 changes: 4 additions & 5 deletions examples/chip-tool/commands/common/CHIPCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ CHIP_ERROR CHIPCommand::Run()
ReturnLogErrorOnFailure(DeviceControllerFactory::GetInstance().SetupCommissioner(commissionerParams, mController));

chip::DeviceLayer::PlatformMgr().ScheduleWork(RunQueuedCommand, reinterpret_cast<intptr_t>(this));
ReturnLogErrorOnFailure(StartWaiting(GetWaitDurationInSeconds()));
ReturnLogErrorOnFailure(StartWaiting(GetWaitDuration()));

Shutdown();

Expand Down Expand Up @@ -108,12 +108,12 @@ static void OnResponseTimeout(chip::System::Layer *, void * appState)
}
#endif // !CONFIG_USE_SEPARATE_EVENTLOOP

CHIP_ERROR CHIPCommand::StartWaiting(uint16_t seconds)
CHIP_ERROR CHIPCommand::StartWaiting(chip::System::Clock::Timeout duration)
{
#if CONFIG_USE_SEPARATE_EVENTLOOP
// ServiceEvents() calls StartEventLoopTask(), which is paired with the StopEventLoopTask() below.
ReturnLogErrorOnFailure(DeviceControllerFactory::GetInstance().ServiceEvents());
auto waitingUntil = std::chrono::system_clock::now() + std::chrono::seconds(seconds);
auto waitingUntil = std::chrono::system_clock::now() + std::chrono::duration_cast<std::chrono::seconds>(duration);
{
std::unique_lock<std::mutex> lk(cvWaitingForResponseMutex);
if (!cvWaitingForResponse.wait_until(lk, waitingUntil, [this]() { return !this->mWaitingForResponse; }))
Expand All @@ -123,8 +123,7 @@ CHIP_ERROR CHIPCommand::StartWaiting(uint16_t seconds)
}
LogErrorOnFailure(chip::DeviceLayer::PlatformMgr().StopEventLoopTask());
#else
ReturnLogErrorOnFailure(
chip::DeviceLayer::SystemLayer().StartTimer(chip::System::Clock::Seconds32(seconds), OnResponseTimeout, this));
ReturnLogErrorOnFailure(chip::DeviceLayer::SystemLayer().StartTimer(duration, OnResponseTimeout, this));
chip::DeviceLayer::PlatformMgr().RunEventLoop();
#endif // CONFIG_USE_SEPARATE_EVENTLOOP

Expand Down
4 changes: 2 additions & 2 deletions examples/chip-tool/commands/common/CHIPCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class CHIPCommand : public Command
virtual CHIP_ERROR RunCommand() = 0;

// Get the wait duration, in seconds, before the command times out.
virtual uint16_t GetWaitDurationInSeconds() const = 0;
virtual chip::System::Clock::Timeout GetWaitDuration() const = 0;

// Shut down the command, in case any work needs to be done after the event
// loop has been stopped.
Expand All @@ -72,7 +72,7 @@ class CHIPCommand : public Command
CHIP_ERROR mCommandExitStatus = CHIP_ERROR_INTERNAL;
chip::Controller::ExampleOperationalCredentialsIssuer mOpCredsIssuer;

CHIP_ERROR StartWaiting(uint16_t seconds);
CHIP_ERROR StartWaiting(chip::System::Clock::Timeout seconds);
void StopWaiting();

#if CONFIG_USE_SEPARATE_EVENTLOOP
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/discover/DiscoverCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class DiscoverCommand : public CHIPCommand, public chip::Controller::DeviceAddre

/////////// CHIPCommand Interface /////////
CHIP_ERROR RunCommand() override;
uint16_t GetWaitDurationInSeconds() const override { return 30; }
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(30); }

virtual CHIP_ERROR RunCommand(NodeId remoteId, uint64_t fabricId) = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@ class DiscoverCommissionablesCommand : public CHIPCommand, public chip::Controll

/////////// CHIPCommand Interface /////////
CHIP_ERROR RunCommand() override;
uint16_t GetWaitDurationInSeconds() const override { return 30; }
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(30); }
};
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@ void DiscoverCommissionersCommand::Shutdown()
}

ChipLogProgress(chipTool, "Total of %d commissioner(s) discovered in %" PRIu16 " sec", commissionerCount,
GetWaitDurationInSeconds());
std::chrono::duration_cast<System::Clock::Seconds16>(GetWaitDuration()).count());
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class DiscoverCommissionersCommand : public CHIPCommand

/////////// CHIPCommand Interface /////////
CHIP_ERROR RunCommand() override;
uint16_t GetWaitDurationInSeconds() const override { return 3; }
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(3); }
void Shutdown() override;

private:
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/pairing/PairingCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class PairingCommand : public CHIPCommand,

/////////// CHIPCommand Interface /////////
CHIP_ERROR RunCommand() override;
uint16_t GetWaitDurationInSeconds() const override { return 120; }
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(120); }
void Shutdown() override;

/////////// DevicePairingDelegate Interface /////////
Expand Down
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/reporting/ReportingCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ReportingCommand : public CHIPCommand

/////////// CHIPCommand Interface /////////
CHIP_ERROR RunCommand() override;
uint16_t GetWaitDurationInSeconds() const override { return UINT16_MAX; }
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(UINT16_MAX); }

virtual void AddReportCallbacks(NodeId remoteId, uint8_t endPointId) = 0;

Expand Down
4 changes: 2 additions & 2 deletions examples/chip-tool/commands/tests/TestCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ void TestCommand::OnWaitForMsFn(chip::System::Layer * systemLayer, void * contex
command->NextTest();
}

CHIP_ERROR TestCommand::WaitForMs(uint32_t ms)
CHIP_ERROR TestCommand::Wait(chip::System::Clock::Timeout duration)
{
return chip::DeviceLayer::SystemLayer().StartTimer(chip::System::Clock::Milliseconds32(ms), OnWaitForMsFn, this);
return chip::DeviceLayer::SystemLayer().StartTimer(duration, OnWaitForMsFn, this);
}

CHIP_ERROR TestCommand::Log(const char * message)
Expand Down
5 changes: 3 additions & 2 deletions examples/chip-tool/commands/tests/TestCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,13 @@ class TestCommand : public CHIPCommand

/////////// CHIPCommand Interface /////////
CHIP_ERROR RunCommand() override;
uint16_t GetWaitDurationInSeconds() const override { return 30; }
chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(30); }

virtual void NextTest() = 0;

/////////// GlobalCommands Interface /////////
CHIP_ERROR WaitForMs(uint32_t ms);
CHIP_ERROR Wait(chip::System::Clock::Timeout ms);
CHIP_ERROR WaitForMs(uint16_t ms) { return Wait(chip::System::Clock::Milliseconds32(ms)); }
CHIP_ERROR Log(const char * message);

protected:
Expand Down

0 comments on commit 1092138

Please sign in to comment.