Conversation
Allow the replay buffer size to be configured server-side, defaulting to 120 if not specified. Valid values are 1-120. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Changed if to while loop when trimming buffer, so that if the max buffer size is reduced below the current buffer count, all excess replays are dropped instead of just one. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a configurable maximum replay buffer size for the Radar SDK. The buffer size can now be configured server-side with a valid range of 1-120, with a default value of 120 (representing one hour of updates).
Changes:
- Added
maxReplayBufferSizeproperty toRadarSdkConfigurationwith validation (range 1-120) and default value of 120 - Updated
RadarReplayBufferto use the configurable buffer size instead of a hardcoded constant - Refactored buffer overflow handling from an if-statement to a while-loop to handle cases where the buffer might be significantly larger than the new limit
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| RadarSDK/RadarSdkConfiguration.h | Added new maxReplayBufferSize property declaration |
| RadarSDK/RadarSdkConfiguration.m | Implemented initialization, parsing, validation, and serialization logic for maxReplayBufferSize |
| RadarSDK/RadarReplayBuffer.m | Updated to use configurable buffer size with improved overflow handling using a while loop |
| .gitmodules | Unrelated whitespace change (tabs to spaces) on line 3 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| NSObject *maxReplayBufferSizeObj = dict[@"maxReplayBufferSize"]; | ||
| if (maxReplayBufferSizeObj && [maxReplayBufferSizeObj isKindOfClass:[NSNumber class]]) { | ||
| int maxReplayBufferSize = [(NSNumber *)maxReplayBufferSizeObj intValue]; | ||
| if (maxReplayBufferSize > 0 && maxReplayBufferSize <= 120) { | ||
| _maxReplayBufferSize = maxReplayBufferSize; | ||
| } | ||
| } |
There was a problem hiding this comment.
The new maxReplayBufferSize configuration parameter lacks test coverage. The existing test_RadarSdkConfiguration test should be extended to verify that maxReplayBufferSize is correctly parsed from the configuration dictionary, validated according to its constraints (range 1-120), and properly serialized back to a dictionary. Additionally, test coverage should be added to verify the behavior in RadarReplayBuffer when different maxReplayBufferSize values are configured.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.