mirror of
https://github.com/espressif/openthread.git
synced 2026-08-29 05:19:54 +00:00
Sven Kirmess
2a8b4a57ed
`RestoreProperties()` re-applies the max power table behind `#if
OPENTHREAD_POSIX_CONFIG_MAX_POWER_TABLE_ENABLE`, but
`radio_spinel.cpp` never sees that macro. It is a POSIX macro, and
this is a platform-agnostic library whose only project config hook is
OPENTHREAD_PROJECT_LIB_CONFIG_FILE, from `OT_LIB_CONFIG`, which is not
set anywhere. `-DOT_POSIX_MAX_POWER_TABLE=ON` puts the definition on
`ot-posix-config`, which only `openthread-posix` links.
So the block is compiled out everywhere, including in
`script/cmake-build posix` and `script/check-posix-pty`, which passes
`max-power-table` in its radio URL. The values are applied at startup
and silently lost on the next RCP reset.
Dropping the guard exposes a second problem. An RCP without
`SPINEL_PROP_PHY_CHAN_MAX_POWER` answers
`SPINEL_STATUS_PROP_NOT_FOUND`, which `spinel_status_to_ot_error()`
maps to `OT_ERROR_NOT_IMPLEMENTED` -- not the `OT_ERROR_NOT_FOUND` the
code tolerates. Every RCP reset would then be fatal. The loop now
warns once, clears `mMaxPowerTableSet` and stops, so a refusing RCP is
not asked again on later recoveries either.
That flag also guards the loop, following the `mTransmitPowerSet` and
`mCcaEnergyDetectThresholdSet` flags beside it, and is set only after
`Set()` succeeds. The loop calls `Set()` directly rather than
`SetChannelMaxTransmitPower()`, as the other restores here do: the
channel range was already checked when the value was configured, and
writing the value back into the table it was just read from would be
circular now that the setter also raises the flag. `MaxPowerTable`
initialises every channel to `kPowerDefault`, so without it the loop
would issue 16 blocking transactions on every restore for everyone,
including those who never pass `max-power-table`. Comparing against
`kPowerDefault` instead would have been cheaper but not equivalent: it
cannot tell an unconfigured channel from one deliberately set to 30
dBm.
With the macro no longer used outside the POSIX platform, enabling it by
default there makes `max-power-table` reachable, like `cca-threshold` and
`fem-lnagain` already are.
Three gtest cases cover it. The fake platform has to implement
`otPlatRadioSetChannelMaxTransmitPower` for them: the weak default in
`radio_platform.cpp` reports `kErrorNotImplemented`, so without an
implementation a test cannot tell a correct restore from one that
never arrived.
shouldRestoreEachChannelWithItsOwnPower three channels, distinct
powers, asserted per channel
after a restore
shouldNotTouchTheRcpWhenNoChannelWasConfigured pins the flag
shouldSurviveAnRcpThatDoesNotImplementIt refusing RCP, must not abort
and must not retry
The first and third fail without the corresponding fix; the second
passes either way, because the removed guard takes the same loop with
it.
Also measured against a simulated RCP behind a socat PTY, with an RCP
reset forced by freezing the RCP process, using
`SPINEL_PROP_MAC_15_4_PANID` as the control for whether a restore ran
at all:
config startup control restored alive
before 16 10x 0 yes
after 16 10x 80 yes
after, no max-power-table in the URL 0 10x 0 yes
old error handling, block on, old RCP 0 6x 0 NO
after, old RCP 0 10x 0 yes
old error handling, block on, capable RCP 16 10x 80 yes
Built and tested with OT_RCP_RESTORATION_MAX_COUNT at both 0 and 2.