espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-08-15 07:07:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

[diag] refactor command processing to remove redundant code (#11055)

Browse Source 
All diagnostic commands (instead of `diag start`) should fail if
device is not in diagnostic mode. Previously it was verified by
each command's process method (with a missing check in
`ProcessEcho` and `ProcessGpio`). This commit moves the check
directly to `ProcessCmd` and cleans up redundant code.

Additionally clean the documentation and align the code as for
some commands `status 0x00` was added on success and for some not.

Move `AppendErrorResult()` to `ProcessCmd()` as well.

Signed-off-by: Maciej Baczmanski <[email protected]>
This commit is contained in:
Maciej Baczmański
2024-12-18 13:52:33 -08:00
committed by GitHub
parent 4e9d362b67
commit 33c9e67a9e
6 changed files with 44 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/diags/README.md
+14 -9
View File
@@ -38,7 +38,7 @@ Start diagnostics mode.
```bash
> diag start
start diagnostics mode
status 0x00
Done
```
### diag channel
@@ -48,6 +48,7 @@ Get the IEEE 802.15.4 Channel value for diagnostics module.
```bash
> diag channel
channel: 11
Done
```
### diag channel \<channel\>
@@ -57,7 +58,7 @@ Set the IEEE 802.15.4 Channel value for diagnostics module.
```bash
> diag channel 11
set channel to 11
status 0x00
Done
```
### diag cw start
@@ -123,6 +124,7 @@ Get the tx power value(dBm) for diagnostics module.
```bash
> diag power
tx power: -10 dBm
Done
```
### diag power \<power\>
@@ -132,7 +134,7 @@ Set the tx power value(dBm) for diagnostics module.
```bash
> diag power -10
set tx power to -10 dBm
status 0x00
Done
```
### diag powersettings
@@ -173,7 +175,7 @@ Send the frame set by `diag frame` if length is omitted. Otherwise overwrite the
```bash
> diag send 20 100
sending 0x14 packet(s), length 0x64
status 0x00
Done
```
### diag repeat \<delay\> [length]
@@ -185,7 +187,7 @@ Send the frame set by `diag frame` if length is omitted. Otherwise overwrite the
```bash
> diag repeat 100 100
sending packets of length 0x64 at the delay of 0x64 ms
status 0x00
Done
```
### diag repeat stop
@@ -195,7 +197,7 @@ Stop repeated packet transmission.
```bash
> diag repeat stop
repeated packet transmission is stopped
status 0x00
Done
```
### diag radio sleep
@@ -205,7 +207,7 @@ Enter radio sleep mode.
```bash
> diag radio sleep
set radio from receive to sleep
status 0x00
Done
```
### diag radio receive
@@ -215,7 +217,7 @@ Set radio from sleep mode to receive mode.
```bash
> diag radio receive
set radio from sleep to receive on channel 11
status 0x00
Done
```
### diag radio receive \[async\] \<number\> \[lpr\]
@@ -245,6 +247,7 @@ Return the state of the radio.
```bash
> diag radio state
sleep
Done
```
### diag radio enable
@@ -312,6 +315,7 @@ received packets: 10
sent packets: 10
first received packet: rssi=-65, lqi=101
last received packet: rssi=-64, lqi=98
Done
```
### diag stats clear
@@ -321,6 +325,7 @@ Clear statistics during diagnostics mode.
```bash
> diag stats clear
stats cleared
Done
```
### diag gpio get \<gpio\>
@@ -384,7 +389,7 @@ first received packet: rssi=-65, lqi=101
last received packet: rssi=-61, lqi=98
stop diagnostics mode
status 0x00
Done
```
### diag rcp
src/core/diags/factory_diags.cpp
+20 -50
View File
@@ -93,7 +93,6 @@ Error Diags::ProcessChannel(uint8_t aArgsLength, char *aArgs[])
otPlatDiagChannelSet(channel);
exit:
AppendErrorResult(error);
return error;
}
@@ -109,7 +108,6 @@ Error Diags::ProcessPower(uint8_t aArgsLength, char *aArgs[])
otPlatDiagTxPowerSet(power);
exit:
AppendErrorResult(error);
return error;
}
@@ -148,7 +146,6 @@ Error Diags::ProcessEcho(uint8_t aArgsLength, char *aArgs[])
}
exit:
AppendErrorResult(error);
return error;
}
@@ -326,7 +323,6 @@ Error Diags::ProcessFrame(uint8_t aArgsLength, char *aArgs[])
mIsTxPacketSet = true;
exit:
AppendErrorResult(error);
return error;
}
@@ -334,8 +330,6 @@ Error Diags::ProcessChannel(uint8_t aArgsLength, char *aArgs[])
{
Error error = kErrorNone;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
if (aArgsLength == 0)
{
Output("channel: %d\r\n", mChannel);
@@ -351,11 +345,10 @@ Error Diags::ProcessChannel(uint8_t aArgsLength, char *aArgs[])
IgnoreError(Get<Radio>().Receive(mChannel));
otPlatDiagChannelSet(mChannel);
Output("set channel to %d\r\nstatus 0x%02x\r\n", mChannel, error);
Output("set channel to %d\r\n", mChannel);
}
exit:
AppendErrorResult(error);
return error;
}
@@ -363,8 +356,6 @@ Error Diags::ProcessPower(uint8_t aArgsLength, char *aArgs[])
{
Error error = kErrorNone;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
if (aArgsLength == 0)
{
Output("tx power: %d dBm\r\n", mTxPower);
@@ -379,11 +370,10 @@ Error Diags::ProcessPower(uint8_t aArgsLength, char *aArgs[])
SuccessOrExit(error = Get<Radio>().SetTransmitPower(mTxPower));
otPlatDiagTxPowerSet(mTxPower);
Output("set tx power to %d dBm\r\nstatus 0x%02x\r\n", mTxPower, error);
Output("set tx power to %d dBm\r\n", mTxPower);
}
exit:
AppendErrorResult(error);
return error;
}
@@ -391,14 +381,13 @@ Error Diags::ProcessRepeat(uint8_t aArgsLength, char *aArgs[])
{
Error error = kErrorNone;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
VerifyOrExit(aArgsLength > 0, error = kErrorInvalidArgs);
if (StringMatch(aArgs[0], "stop"))
{
otPlatAlarmMilliStop(&GetInstance());
mRepeatActive = false;
Output("repeated packet transmission is stopped\r\nstatus 0x%02x\r\n", error);
Output("repeated packet transmission is stopped\r\n");
}
else
{
@@ -431,12 +420,11 @@ Error Diags::ProcessRepeat(uint8_t aArgsLength, char *aArgs[])
mRepeatActive = true;
uint32_t now = otPlatAlarmMilliGetNow();
otPlatAlarmMilliStartAt(&GetInstance(), now, mTxPeriod);
Output("sending packets of length %#x at the delay of %#x ms\r\nstatus 0x%02x\r\n", static_cast<int>(mTxLen),
static_cast<int>(mTxPeriod), error);
Output("sending packets of length %#x at the delay of %#x ms\r\n", static_cast<int>(mTxLen),
static_cast<int>(mTxPeriod));
}
exit:
AppendErrorResult(error);
return error;
}
@@ -446,7 +434,6 @@ Error Diags::ProcessSend(uint8_t aArgsLength, char *aArgs[])
uint32_t txPackets;
uint8_t txLength;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
VerifyOrExit(aArgsLength >= 1, error = kErrorInvalidArgs);
SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint32(aArgs[0], txPackets));
@@ -470,12 +457,10 @@ Error Diags::ProcessSend(uint8_t aArgsLength, char *aArgs[])
VerifyOrExit(txLength >= OT_RADIO_FRAME_MIN_SIZE, error = kErrorInvalidArgs);
mTxLen = txLength;
Output("sending %#x packet(s), length %#x\r\nstatus 0x%02x\r\n", static_cast<int>(mTxPackets),
static_cast<int>(mTxLen), error);
Output("sending %#x packet(s), length %#x\r\n", static_cast<int>(mTxPackets), static_cast<int>(mTxLen));
TransmitPacket();
exit:
AppendErrorResult(error);
return error;
}
@@ -501,10 +486,9 @@ Error Diags::ProcessStart(uint8_t aArgsLength, char *aArgs[])
SuccessOrExit(error = Get<Radio>().SetTransmitPower(mTxPower));
otPlatDiagModeSet(true);
mStats.Clear();
Output("start diagnostics mode\r\nstatus 0x%02x\r\n", error);
Output("start diagnostics mode\r\n");
exit:
AppendErrorResult(error);
return error;
}
@@ -512,8 +496,6 @@ Error Diags::ProcessStats(uint8_t aArgsLength, char *aArgs[])
{
Error error = kErrorNone;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
if ((aArgsLength == 1) && StringMatch(aArgs[0], "clear"))
{
mStats.Clear();
@@ -531,7 +513,6 @@ Error Diags::ProcessStats(uint8_t aArgsLength, char *aArgs[])
}
exit:
AppendErrorResult(error);
return error;
}
@@ -540,10 +521,6 @@ Error Diags::ProcessStop(uint8_t aArgsLength, char *aArgs[])
OT_UNUSED_VARIABLE(aArgsLength);
OT_UNUSED_VARIABLE(aArgs);
Error error = kErrorNone;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
otPlatAlarmMilliStop(&GetInstance());
otPlatDiagModeSet(false);
Get<Radio>().SetPromiscuous(false);
@@ -552,14 +529,12 @@ Error Diags::ProcessStop(uint8_t aArgsLength, char *aArgs[])
Output("received packets: %d\r\nsent packets: %d\r\n"
"first received packet: rssi=%d, lqi=%d\r\n"
"last received packet: rssi=%d, lqi=%d\r\n"
"\nstop diagnostics mode\r\nstatus 0x%02x\r\n",
"\nstop diagnostics mode\r\n",
static_cast<int>(mStats.mReceivedPackets), static_cast<int>(mStats.mSentPackets),
static_cast<int>(mStats.mFirstRssi), static_cast<int>(mStats.mFirstLqi), static_cast<int>(mStats.mLastRssi),
static_cast<int>(mStats.mLastLqi), error);
static_cast<int>(mStats.mLastLqi));
exit:
AppendErrorResult(error);
return error;
return kErrorNone;
}
void Diags::TransmitPacket(void)
@@ -635,13 +610,12 @@ Error Diags::ProcessRadio(uint8_t aArgsLength, char *aArgs[])
{
Error error = kErrorInvalidArgs;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
VerifyOrExit(aArgsLength > 0, error = kErrorInvalidArgs);
if (StringMatch(aArgs[0], "sleep"))
{
SuccessOrExit(error = Get<Radio>().Sleep());
Output("set radio from receive to sleep \r\nstatus 0x%02x\r\n", error);
Output("set radio from receive to sleep \r\n");
}
else if (StringMatch(aArgs[0], "receive"))
{
@@ -653,7 +627,7 @@ Error Diags::ProcessRadio(uint8_t aArgsLength, char *aArgs[])
if (aArgsLength == 0)
{
SuccessOrExit(error = RadioReceive());
Output("set radio from sleep to receive on channel %d\r\nstatus 0x%02x\r\n", mChannel, error);
Output("set radio from sleep to receive on channel %d\r\n", mChannel);
ExitNow();
}
@@ -723,7 +697,6 @@ Error Diags::ProcessRadio(uint8_t aArgsLength, char *aArgs[])
}
exit:
AppendErrorResult(error);
return error;
}
@@ -840,7 +813,6 @@ Error Diags::ProcessContinuousWave(uint8_t aArgsLength, char *aArgs[])
{
Error error = kErrorInvalidArgs;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
VerifyOrExit(aArgsLength > 0, error = kErrorInvalidArgs);
if (StringMatch(aArgs[0], "start"))
@@ -853,7 +825,6 @@ Error Diags::ProcessContinuousWave(uint8_t aArgsLength, char *aArgs[])
}
exit:
AppendErrorResult(error);
return error;
}
@@ -861,7 +832,6 @@ Error Diags::ProcessStream(uint8_t aArgsLength, char *aArgs[])
{
Error error = kErrorInvalidArgs;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
VerifyOrExit(aArgsLength > 0, error = kErrorInvalidArgs);
if (StringMatch(aArgs[0], "start"))
@@ -874,7 +844,6 @@ Error Diags::ProcessStream(uint8_t aArgsLength, char *aArgs[])
}
exit:
AppendErrorResult(error);
return error;
}
@@ -892,8 +861,6 @@ Error Diags::ProcessPowerSettings(uint8_t aArgsLength, char *aArgs[])
uint8_t channel;
PowerSettings powerSettings;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
if (aArgsLength == 0)
{
bool isPrePowerSettingsValid = false;
@@ -936,7 +903,6 @@ Error Diags::ProcessPowerSettings(uint8_t aArgsLength, char *aArgs[])
}
exit:
AppendErrorResult(error);
return error;
}
@@ -951,8 +917,6 @@ Error Diags::ProcessRawPowerSetting(uint8_t aArgsLength, char *aArgs[])
Error error = kErrorInvalidArgs;
RawPowerSetting setting;
VerifyOrExit(otPlatDiagModeGet(), error = kErrorInvalidState);
if (aArgsLength == 0)
{
SuccessOrExit(error = GetRawPowerSetting(setting));
@@ -974,7 +938,6 @@ Error Diags::ProcessRawPowerSetting(uint8_t aArgsLength, char *aArgs[])
}
exit:
AppendErrorResult(error);
return error;
}
@@ -1024,7 +987,6 @@ Error Diags::ProcessGpio(uint8_t aArgsLength, char *aArgs[])
}
exit:
AppendErrorResult(error);
return error;
}
@@ -1112,6 +1074,12 @@ Error Diags::ProcessCmd(uint8_t aArgsLength, char *aArgs[])
ExitNow();
}
if (!otPlatDiagModeGet() && !StringMatch(aArgs[0], "start"))
{
Output("diagnostics mode is disabled\r\n");
ExitNow(error = kErrorInvalidState);
}
for (const Command &command : sCommands)
{
if (StringMatch(aArgs[0], command.mName))
@@ -1131,6 +1099,8 @@ exit:
Output("diag feature '%s' is not supported\r\n", aArgs[0]);
}
AppendErrorResult(error);
return error;
}
tests/scripts/expect/cli-diags.exp
-12
View File
@@ -36,12 +36,10 @@ spawn_node 2
switch_node 1
send "diag start\n"
expect "start diagnostics mode"
expect "status 0x00"
expect_line "Done"
send "diag channel 11\n"
expect "set channel to 11"
expect "status 0x00"
expect_line "Done"
send "diag stats clear\n"
@@ -52,12 +50,10 @@ switch_node 2
send "diag start\n"
expect "start diagnostics mode"
expect "status 0x00"
expect_line "Done"
send "diag channel 11\n"
expect "set channel to 11"
expect "status 0x00"
expect_line "Done"
send "diag stats clear\n"
@@ -66,7 +62,6 @@ expect_line "Done"
send "diag send 10 100\n"
expect "sending 0xa packet(s), length 0x64"
expect "status 0x00"
expect_line "Done"
sleep 2
@@ -95,12 +90,10 @@ switch_node 2
send "diag repeat 20 100\n"
expect "sending packets of length 0x64 at the delay of 0x14 ms"
expect "status 0x00"
expect_line "Done"
sleep 1
send "diag repeat stop\n"
expect "repeated packet transmission is stopped"
expect "status 0x00"
expect_line "Done"
switch_node 1
@@ -123,7 +116,6 @@ spawn_node 1
send "diag start\n"
expect "start diagnostics mode"
expect "status 0x00"
expect_line "Done"
send_user "input too short test\n"
@@ -178,17 +170,14 @@ expect "Done"
send "diag channel 11\n"
expect "set channel to 11"
expect "status 0x00"
expect_line "Done"
send "diag power 10\n"
expect "set tx power to 10 dBm"
expect "status 0x00"
expect_line "Done"
send "diag radio sleep\n"
expect "set radio from receive to sleep"
expect "status 0x00"
expect_line "Done"
send "diag radio state\n"
@@ -197,7 +186,6 @@ expect_line "Done"
send "diag radio receive\n"
expect "set radio from sleep to receive on channel 11"
expect "status 0x00"
expect_line "Done"
send "diag radio state\n"
tests/scripts/expect/posix-diag-rcp.exp
+3 -2
View File
@@ -33,10 +33,11 @@ spawn_node 1
send "diag rcp\n"
expect "diagnostics mode is disabled"
expect_line "Done"
expect_line "failed"
expect_line "status 0xd"
expect_line "Error 13: InvalidState"
send "diag start\n"
expect "start diagnostics mode"
expect "status 0x00"
expect_line "Done"
send "diag rcp\n"
expect "diagnostics mode is enabled"
tests/scripts/expect/posix-power-calibration.exp
-2
View File
@@ -33,7 +33,6 @@ spawn_node 1
send "diag start\n"
expect "start diagnostics mode"
expect "status 0x00"
expect_line "Done"
send "diag powersettings 11\n"
@@ -54,7 +53,6 @@ expect_line "Done"
send "diag start\n"
expect "start diagnostics mode"
expect "status 0x00"
expect_line "Done"
send "diag powersettings 11\n"
tests/scripts/thread-cert/test_diag.py
+7 -7
View File
@@ -48,9 +48,9 @@ class TestDiag(thread_cert.TestCase):
('diag invalid test\n', 'diag feature \'invalid\' is not supported'),
('diag', 'diagnostics mode is enabled\r\n'),
('diag channel 10', 'failed\r\nstatus 0x7\r\n'),
('diag channel 11', 'set channel to 11\r\nstatus 0x00\r\n'),
('diag channel 11', 'set channel to 11\r\n'),
('diag channel', 'channel: 11\r\n'),
('diag power -10', 'set tx power to -10 dBm\r\nstatus 0x00\r\n'),
('diag power -10', 'set tx power to -10 dBm\r\n'),
('diag power', 'tx power: -10 dBm\r\n'),
(
'diag stats',
@@ -60,26 +60,26 @@ class TestDiag(thread_cert.TestCase):
),
(
'diag send 20 100',
r'sending 0x14 packet\(s\), length 0x64\r\nstatus 0x00\r\n',
r'sending 0x14 packet\(s\), length 0x64\r\n',
),
(
' diag \t send \t 20\t100',
r'sending 0x14 packet\(s\), length 0x64\r\nstatus 0x00\r\n',
r'sending 0x14 packet\(s\), length 0x64\r\n',
),
(
'diag repeat 100 100',
'sending packets of length 0x64 at the delay of 0x64 ms\r\nstatus 0x00\r\n',
'sending packets of length 0x64 at the delay of 0x64 ms\r\n',
),
(
'diag repeat stop',
'repeated packet transmission is stopped\r\nstatus 0x00\r\n',
'repeated packet transmission is stopped\r\n',
),
(
'diag stop',
r'received packets: 0\r\nsent packets: ([1-9]\d*)\r\n'
'first received packet: rssi=0, lqi=0\r\n'
'last received packet: rssi=0, lqi=0\r\n\n'
r'stop diagnostics mode\r\nstatus 0x00\r\n',
r'stop diagnostics mode\r\n',
),
('diag', 'diagnostics mode is disabled\r\n'),
(