DEV-552: Fix for instream response & WDT trigger when streaming ExG docked. #70
Conversation
rameshchh
changed the title
| } | ||
| else //if start sd logging not triggered on undock. | ||
| { | ||
| ShimBt_instreamStatusRespSend(); |
There was a problem hiding this comment.
Does this response needed always ? or the source needs to be checked ?
using
void ShimBt_instreamStatusRespSendIfNotBtCmd(void)
this function ??
There was a problem hiding this comment.
ShimBt_instreamStatusRespSend() attempts to sent response only if BT is connected.
| Board_setSdPower(0); | ||
|
|
||
| ShimBt_instreamStatusRespSend(); | ||
| //ShimBt_instreamStatusRespSend(); |
There was a problem hiding this comment.
if this is moved to ShimSens_startLoggingIfUndockStartEnabled, then in the case that there's no SD card, Consensys won't be told via BT that the device has been undocked.
Should we move it instead to the end of this function outside of any if statements?
There was a problem hiding this comment.
Pull request overview
This PR addresses two critical issues in the Shimmer sensor firmware related to docking/undocking events and streaming operations for S3 and S3R devices:
- Fixes instream status response timing during undock events to ensure proper communication with BT hosts
- Resolves a Watchdog Timer (WDT) trigger issue on S3 devices when docked and streaming is stopped
Key Changes
- Relocated the instream status response logic to only send when SD logging doesn't auto-start on undock
- Moved
DockUart_init()call after peripheral shutdown to prevent WDT timeout - Added
volatilequalifier to shared state variable for thread-safety
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| log_and_stream_common.c | Moved ShimSens_startLoggingIfUndockStartEnabled() call from LogAndStream_syncConfigAndCalibOnSd() to LogAndStream_setupUndock() and commented out unconditional instream response send |
| Sensing/shimmer_sensing.c | Relocated DockUart_init() after ShimSens_stopPeripherals() to fix WDT trigger; integrated instream response into undock start logic; refactored to use ShimConfig_isExpansionBoardPwrEnabled() function |
| Comms/shimmer_bt_uart.c | Added volatile qualifier to sensingStateChangeFromBtCmd for thread-safe access in interrupt contexts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
| ShimSens_startLoggingIfUndockStartEnabled(); | ||
| } |
There was a problem hiding this comment.
When there is no SD card in the slot during undock (LogAndStream_checkSdInSlot() returns false at line 268), the instream status response will not be sent at all, since ShimSens_startLoggingIfUndockStartEnabled() is only called inside the if (LogAndStream_checkSdInSlot()) block at line 288.
This means the BT host won't receive a status update after an undock event when no SD card is present. Consider calling ShimBt_instreamStatusRespSend() in the else branch after line 289 to ensure the status response is always sent on undock:
if (LogAndStream_checkSdInSlot())
{
// existing code...
ShimSens_startLoggingIfUndockStartEnabled();
}
else
{
ShimBt_instreamStatusRespSend();
}| } | |
| } | |
| else | |
| { | |
| ShimBt_instreamStatusRespSend(); | |
| } |
| if (!shimmerStatus.sensing) | ||
| { | ||
| platform_delayMs(120); //120ms | ||
| LogAndStream_syncConfigAndCalibOnSd(); | ||
| } | ||
| else | ||
| { | ||
| LogAndStream_setSdInfoSyncDelayed(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm afraid here of the device starting logging before it's saved configuration to SD card. If the device is already sensing, maybe it's safe not to read ram again inside LogAndStream_syncConfigAndCalibOnSd and don't reconfigure channels, just saved the config/calib files to SD card and start logging if needed afterwards. I can't think of the need for the delayed info sync unless the SD card has newer config compared to what was configured over BT and is in use while currently sensing
Fix 1: Instream response when undock event to start logging for both S3 and S3R.
Fix 2: WDT triggering on S3 when sensor is docked and stop streaming.