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

Make platform PlatformManager::PostEvent() return status #9573

Merged
merged 6 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
17 changes: 14 additions & 3 deletions src/include/platform/PlatformManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ class PlatformManager
* processing, the event might get dispatched (on the work item processing
* thread) before PostEvent returns.
*/
void PostEvent(const ChipDeviceEvent * event);
[[nodiscard]] CHIP_ERROR PostEvent(const ChipDeviceEvent * event);
CHIP_ERROR PostEventLoggingErrors(const ChipDeviceEvent * event);
void DispatchEvent(const ChipDeviceEvent * event);
CHIP_ERROR StartChipTimer(uint32_t durationMS);

Expand Down Expand Up @@ -354,9 +355,19 @@ inline void PlatformManager::UnlockChipStack()
static_cast<ImplClass *>(this)->_UnlockChipStack();
}

inline void PlatformManager::PostEvent(const ChipDeviceEvent * event)
inline CHIP_ERROR PlatformManager::PostEvent(const ChipDeviceEvent * event)
woody-apple marked this conversation as resolved.
Show resolved Hide resolved
{
static_cast<ImplClass *>(this)->_PostEvent(event);
return static_cast<ImplClass *>(this)->_PostEvent(event);
}

inline CHIP_ERROR PlatformManager::PostEventLoggingErrors(const ChipDeviceEvent * event)
woody-apple marked this conversation as resolved.
Show resolved Hide resolved
{
CHIP_ERROR status = static_cast<ImplClass *>(this)->_PostEvent(event);
if (status != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Failed to post event %d: %" CHIP_ERROR_FORMAT, static_cast<int>(event->Type), status.Format());
}
return status;
}

inline void PlatformManager::DispatchEvent(const ChipDeviceEvent * event)
Expand Down
13 changes: 10 additions & 3 deletions src/include/platform/internal/GenericConfigurationManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -781,11 +781,14 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_StoreServiceProvisioning
template <class ImplClass>
CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_ClearServiceProvisioningData()
{
CHIP_ERROR err = CHIP_NO_ERROR;

Impl()->ClearConfigValue(ImplClass::kConfigKey_ServiceId);
Impl()->ClearConfigValue(ImplClass::kConfigKey_ServiceConfig);
Impl()->ClearConfigValue(ImplClass::kConfigKey_PairedAccountId);

// TODO: Move these behaviors out of configuration manager.
// Also, should the flags be cleared even if the corresponding notification fails to post?

// If necessary, post an event alerting other subsystems to the change in
// the account pairing state.
Expand All @@ -794,7 +797,7 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_ClearServiceProvisioning
ChipDeviceEvent event;
event.Type = DeviceEventType::kAccountPairingChange;
event.AccountPairingChange.IsPairedToAccount = false;
PlatformMgr().PostEvent(&event);
err = PlatformMgr().PostEvent(&event);
}

// If necessary, post an event alerting other subsystems to the change in
Expand All @@ -805,13 +808,17 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_ClearServiceProvisioning
event.Type = DeviceEventType::kServiceProvisioningChange;
event.ServiceProvisioningChange.IsServiceProvisioned = false;
event.ServiceProvisioningChange.ServiceConfigUpdated = false;
PlatformMgr().PostEvent(&event);
CHIP_ERROR postError = PlatformMgr().PostEvent(&event);
if (err == CHIP_NO_ERROR)
{
err = postError;
}
}

mFlags.Clear(Flags::kIsServiceProvisioned);
mFlags.Clear(Flags::kIsPairedToAccount);

return CHIP_NO_ERROR;
return err;
}

template <class ImplClass>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,11 @@ void GenericConnectivityManagerImpl_Thread<ImplClass>::UpdateServiceConnectivity
event.ServiceConnectivityChange.ViaThread.Result =
(haveServiceConnectivity) ? kConnectivity_Established : kConnectivity_Lost;
event.ServiceConnectivityChange.Overall.Result = event.ServiceConnectivityChange.ViaThread.Result;
PlatformMgr().PostEvent(&event);
CHIP_ERROR status = PlatformMgr().PostEvent(&event);
if (status != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Failed to post thread connectivity change: %" CHIP_ERROR_FORMAT, status.Format());
}
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/include/platform/internal/GenericPlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,11 @@ void GenericPlatformManagerImpl<ImplClass>::_ScheduleWork(AsyncWorkFunct workFun
event.CallWorkFunct.WorkFunct = workFunct;
event.CallWorkFunct.Arg = arg;

Impl()->PostEvent(&event);
CHIP_ERROR status = Impl()->PostEvent(&event);
if (status != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Failed to schedule work: %" CHIP_ERROR_FORMAT, status.Format());
}
}

template <class ImplClass>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,19 @@ void GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_UnlockChipStack(void)
}

template <class ImplClass>
void GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_PostEvent(const ChipDeviceEvent * event)
CHIP_ERROR GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_PostEvent(const ChipDeviceEvent * event)
{
if (mChipEventQueue != NULL)
if (mChipEventQueue == NULL)
{
if (!xQueueSend(mChipEventQueue, event, 1))
{
ChipLogError(DeviceLayer, "Failed to post event to CHIP Platform event queue");
}
return CHIP_ERROR_INTERNAL;
}
BaseType_t status = xQueueSend(mChipEventQueue, event, 1);
if (status != pdTRUE)
{
ChipLogError(DeviceLayer, "Failed to post event to CHIP Platform event queue");
return CHIP_ERROR(chip::ChipError::Range::kOS, status);
}
return CHIP_NO_ERROR;
}

template <class ImplClass>
Expand Down Expand Up @@ -218,7 +222,7 @@ CHIP_ERROR GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_StartChipTimer(uint3
{
ChipDeviceEvent event;
event.Type = DeviceEventType::kNoOp;
Impl()->PostEvent(&event);
ReturnErrorOnFailure(Impl()->PostEvent(&event));
}

return CHIP_NO_ERROR;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class GenericPlatformManagerImpl_FreeRTOS : public GenericPlatformManagerImpl<Im
void _LockChipStack(void);
bool _TryLockChipStack(void);
void _UnlockChipStack(void);
void _PostEvent(const ChipDeviceEvent * event);
CHIP_ERROR _PostEvent(const ChipDeviceEvent * event);
void _RunEventLoop(void);
CHIP_ERROR _StartEventLoopTask(void);
CHIP_ERROR _StopEventLoopTask();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,12 @@ CHIP_ERROR GenericPlatformManagerImpl_POSIX<ImplClass>::_StartChipTimer(int64_t
}

template <class ImplClass>
void GenericPlatformManagerImpl_POSIX<ImplClass>::_PostEvent(const ChipDeviceEvent * event)
CHIP_ERROR GenericPlatformManagerImpl_POSIX<ImplClass>::_PostEvent(const ChipDeviceEvent * event)
{
mChipEventQueue.Push(*event);

SystemLayer.Signal(); // Trigger wake select on CHIP thread
return CHIP_NO_ERROR;
}

template <class ImplClass>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class GenericPlatformManagerImpl_POSIX : public GenericPlatformManagerImpl<ImplC
void _LockChipStack();
bool _TryLockChipStack();
void _UnlockChipStack();
void _PostEvent(const ChipDeviceEvent * event);
CHIP_ERROR _PostEvent(const ChipDeviceEvent * event);
void _RunEventLoop();
CHIP_ERROR _StartEventLoopTask();
CHIP_ERROR _StopEventLoopTask();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
// from which the GenericPlatformManagerImpl_Zephyr<> template inherits.
#include <platform/internal/GenericPlatformManagerImpl.cpp>

#include <system/SystemError.h>
#include <system/SystemLayer.h>

#define DEFAULT_MIN_SLEEP_PERIOD (60 * 60 * 24 * 30) // Month [sec]
Expand Down Expand Up @@ -100,15 +101,19 @@ CHIP_ERROR GenericPlatformManagerImpl_Zephyr<ImplClass>::_Shutdown(void)
}

template <class ImplClass>
void GenericPlatformManagerImpl_Zephyr<ImplClass>::_PostEvent(const ChipDeviceEvent * event)
CHIP_ERROR GenericPlatformManagerImpl_Zephyr<ImplClass>::_PostEvent(const ChipDeviceEvent * event)
{
// For some reasons mentioned in https://github.com/zephyrproject-rtos/zephyr/issues/22301
// k_msgq_put takes `void*` instead of `const void*`. Nonetheless, it should be safe to
// const_cast here and there are components in Zephyr itself which do the same.
if (k_msgq_put(&mChipEventQueue, const_cast<ChipDeviceEvent *>(event), K_NO_WAIT) == 0)
SystemLayer.Signal(); // Trigger wake on CHIP thread
else
int status = k_msgq_put(&mChipEventQueue, const_cast<ChipDeviceEvent *>(event), K_NO_WAIT);
if (status != 0)
{
ChipLogError(DeviceLayer, "Failed to post event to CHIP Platform event queue");
return System::MapErrorZephyr(status);
}
SystemLayer.Signal(); // Trigger wake on CHIP thread
return CHIP_NO_ERROR;
}

template <class ImplClass>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class GenericPlatformManagerImpl_Zephyr : public GenericPlatformManagerImpl<Impl
void _LockChipStack(void);
bool _TryLockChipStack(void);
void _UnlockChipStack(void);
void _PostEvent(const ChipDeviceEvent * event);
CHIP_ERROR _PostEvent(const ChipDeviceEvent * event);
void _RunEventLoop(void);
CHIP_ERROR _StartEventLoopTask(void);
CHIP_ERROR _StopEventLoopTask();
Expand Down
3 changes: 2 additions & 1 deletion src/platform/Darwin/PlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,13 @@ CHIP_ERROR PlatformManagerImpl::_Shutdown()
return GenericPlatformManagerImpl<ImplClass>::_Shutdown();
}

void PlatformManagerImpl::_PostEvent(const ChipDeviceEvent * event)
CHIP_ERROR PlatformManagerImpl::_PostEvent(const ChipDeviceEvent * event)
{
const ChipDeviceEvent eventCopy = *event;
dispatch_async(mWorkQueue, ^{
Impl()->DispatchEvent(&eventCopy);
});
return CHIP_NO_ERROR;
}

} // namespace DeviceLayer
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Darwin/PlatformManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
void _LockChipStack(){};
bool _TryLockChipStack() { return false; };
void _UnlockChipStack(){};
void _PostEvent(const ChipDeviceEvent * event);
CHIP_ERROR _PostEvent(const ChipDeviceEvent * event);

#if CHIP_STACK_LOCK_TRACKING_ENABLED
bool _IsChipStackLockedByCurrentThread() const { return false; };
Expand Down
9 changes: 6 additions & 3 deletions src/platform/DeviceControlServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ void DeviceControlServer::CommissioningFailedTimerComplete()
ChipDeviceEvent event;
event.Type = DeviceEventType::kCommissioningComplete;
event.CommissioningComplete.status = CHIP_ERROR_TIMEOUT;
PlatformMgr().PostEvent(&event);
CHIP_ERROR status = PlatformMgr().PostEvent(&event);
if (status != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Failed to post commissioning complete: %" CHIP_ERROR_FORMAT, status.Format());
}
}

CHIP_ERROR DeviceControlServer::ArmFailSafe(uint16_t expiryLengthSeconds)
Expand All @@ -68,8 +72,7 @@ CHIP_ERROR DeviceControlServer::CommissioningComplete()
ChipDeviceEvent event;
event.Type = DeviceEventType::kCommissioningComplete;
event.CommissioningComplete.status = CHIP_NO_ERROR;
PlatformMgr().PostEvent(&event);
return CHIP_NO_ERROR;
return PlatformMgr().PostEvent(&event);
}

CHIP_ERROR DeviceControlServer::SetRegulatoryConfig(uint8_t location, const char * countryCode, uint64_t breadcrumb)
Expand Down
16 changes: 8 additions & 8 deletions src/platform/EFR32/BLEManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ void BLEManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
ChipLogProgress(DeviceLayer, "_OnPlatformEvent kCHIPoBLESubscribe");
HandleSubscribeReceived(event->CHIPoBLESubscribe.ConId, &CHIP_BLE_SVC_ID, &ChipUUID_CHIPoBLEChar_TX);
connEstEvent.Type = DeviceEventType::kCHIPoBLEConnectionEstablished;
PlatformMgr().PostEvent(&connEstEvent);
PlatformMgr().PostEventLoggingErrors(&connEstEvent);
}
break;

Expand Down Expand Up @@ -523,7 +523,7 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU
{
event.Type = DeviceEventType::kCHIPoBLENotifyConfirm;
event.CHIPoBLENotifyConfirm.ConId = conId;
PlatformMgr().PostEvent(&event);
err = PlatformMgr().PostEvent(&event);
}

exit:
Expand Down Expand Up @@ -881,7 +881,7 @@ void BLEManagerImpl::HandleConnectionCloseEvent(volatile sl_bt_msg_t * evt)

ChipLogProgress(DeviceLayer, "BLE GATT connection closed (con %u, reason %u)", connHandle, conn_evt->reason);

PlatformMgr().PostEvent(&event);
PlatformMgr().PostEventLoggingErrors(&event);

// Arrange to re-enable connectable advertising in case it was disabled due to the
// maximum connection limit being reached.
Expand Down Expand Up @@ -931,7 +931,7 @@ void BLEManagerImpl::HandleTXCharCCCDWrite(volatile sl_bt_msg_t * evt)
{
event.Type = DeviceEventType::kCHIPoBLESubscribe;
event.CHIPoBLESubscribe.ConId = evt->data.evt_gatt_server_user_write_request.connection;
PlatformMgr().PostEvent(&event);
err = PlatformMgr().PostEvent(&event);
}
}
}
Expand All @@ -940,7 +940,7 @@ void BLEManagerImpl::HandleTXCharCCCDWrite(volatile sl_bt_msg_t * evt)
bleConnState->subscribed = 0;
event.Type = DeviceEventType::kCHIPoBLEUnsubscribe;
event.CHIPoBLESubscribe.ConId = evt->data.evt_gatt_server_user_write_request.connection;
PlatformMgr().PostEvent(&event);
err = PlatformMgr().PostEvent(&event);
}

exit:
Expand Down Expand Up @@ -970,7 +970,7 @@ void BLEManagerImpl::HandleRXCharWrite(volatile sl_bt_msg_t * evt)
event.Type = DeviceEventType::kCHIPoBLEWriteReceived;
event.CHIPoBLEWriteReceived.ConId = evt->data.evt_gatt_server_user_write_request.connection;
event.CHIPoBLEWriteReceived.Data = std::move(buf).UnsafeRelease();
PlatformMgr().PostEvent(&event);
err = PlatformMgr().PostEvent(&event);
}

exit:
Expand All @@ -996,7 +996,7 @@ void BLEManagerImpl::HandleTxConfirmationEvent(BLE_CONNECTION_OBJECT conId)

event.Type = DeviceEventType::kCHIPoBLEIndicateConfirm;
event.CHIPoBLEIndicateConfirm.ConId = conId;
PlatformMgr().PostEvent(&event);
PlatformMgr().PostEventLoggingErrors(&event);
}

void BLEManagerImpl::HandleSoftTimerEvent(volatile sl_bt_msg_t * evt)
Expand All @@ -1011,7 +1011,7 @@ void BLEManagerImpl::HandleSoftTimerEvent(volatile sl_bt_msg_t * evt)
event.CHIPoBLEConnectionError.ConId = mIndConfId[evt->data.evt_system_soft_timer.handle];
sInstance.mIndConfId[evt->data.evt_system_soft_timer.handle] = kUnusedIndex;
event.CHIPoBLEConnectionError.Reason = BLE_ERROR_CHIPOBLE_PROTOCOL_ABORT;
PlatformMgr().PostEvent(&event);
PlatformMgr().PostEventLoggingErrors(&event);
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/platform/ESP32/ConnectivityManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ void ConnectivityManagerImpl::OnStationConnected()
ChipDeviceEvent event;
event.Type = DeviceEventType::kWiFiConnectivityChange;
event.WiFiConnectivityChange.Result = kConnectivity_Established;
PlatformMgr().PostEvent(&event);
PlatformMgr().PostEventLoggingErrors(&event);

UpdateInternetConnectivityState();
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
}
Expand All @@ -680,7 +680,7 @@ void ConnectivityManagerImpl::OnStationDisconnected()
ChipDeviceEvent event;
event.Type = DeviceEventType::kWiFiConnectivityChange;
event.WiFiConnectivityChange.Result = kConnectivity_Lost;
PlatformMgr().PostEvent(&event);
PlatformMgr().PostEventLoggingErrors(&event);

UpdateInternetConnectivityState();
}
Expand Down Expand Up @@ -950,7 +950,7 @@ void ConnectivityManagerImpl::UpdateInternetConnectivityState(void)
event.InternetConnectivityChange.IPv4 = GetConnectivityChange(hadIPv4Conn, haveIPv4Conn);
event.InternetConnectivityChange.IPv6 = GetConnectivityChange(hadIPv6Conn, haveIPv6Conn);
addr.ToString(event.InternetConnectivityChange.address);
PlatformMgr().PostEvent(&event);
PlatformMgr().PostEventLoggingErrors(&event);

if (haveIPv4Conn != hadIPv4Conn)
{
Expand Down Expand Up @@ -981,7 +981,7 @@ void ConnectivityManagerImpl::OnStationIPv4AddressAvailable(const ip_event_got_i
ChipDeviceEvent event;
event.Type = DeviceEventType::kInterfaceIpAddressChanged;
event.InterfaceIpAddressChanged.Type = InterfaceIpChangeType::kIpV4_Assigned;
PlatformMgr().PostEvent(&event);
PlatformMgr().PostEventLoggingErrors(&event);
}

void ConnectivityManagerImpl::OnStationIPv4AddressLost(void)
Expand All @@ -995,7 +995,7 @@ void ConnectivityManagerImpl::OnStationIPv4AddressLost(void)
ChipDeviceEvent event;
event.Type = DeviceEventType::kInterfaceIpAddressChanged;
event.InterfaceIpAddressChanged.Type = InterfaceIpChangeType::kIpV4_Lost;
PlatformMgr().PostEvent(&event);
PlatformMgr().PostEventLoggingErrors(&event);
}

void ConnectivityManagerImpl::OnIPv6AddressAvailable(const ip_event_got_ip6_t & got_ip)
Expand All @@ -1014,7 +1014,7 @@ void ConnectivityManagerImpl::OnIPv6AddressAvailable(const ip_event_got_ip6_t &
ChipDeviceEvent event;
event.Type = DeviceEventType::kInterfaceIpAddressChanged;
event.InterfaceIpAddressChanged.Type = InterfaceIpChangeType::kIpV6_Assigned;
PlatformMgr().PostEvent(&event);
PlatformMgr().PostEventLoggingErrors(&event);
}

void ConnectivityManagerImpl::RefreshMessageLayer(void) {}
Expand Down
2 changes: 1 addition & 1 deletion src/platform/ESP32/PlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ void PlatformManagerImpl::HandleESPSystemEvent(void * arg, esp_event_base_t even
}
}

sInstance.PostEvent(&event);
sInstance.PostEventLoggingErrors(&event);
}

} // namespace DeviceLayer
Expand Down
Loading