Fix duplicate ATIS subscriber notifications #115
Conversation
WalkthroughA modification has been made in the Changes
Sequence Diagram(s)sequenceDiagram
participant VM as AtisStationViewModel
participant METAR as METAR Response
participant Sub as Subscriber
Note over VM: OnMetarResponseReceived triggered
METAR->>VM: Receive METAR data
VM->>VM: Check if initial METAR has been processed
alt First METAR Response
VM->>Sub: Send notification
else Duplicate METAR Response
VM-->>Sub: Skip notification
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
vATIS.Desktop/Ui/ViewModels/AtisStationViewModel.cs (2)
1102-1116: Add error handling for UI thread property updates.The UI thread property updates lack error handling, which could lead to a deadlock if an exception occurs during the updates. The
TaskCompletionSourcewould never complete in this case.Apply this diff to add error handling:
var propertyUpdates = new TaskCompletionSource(); Dispatcher.UIThread.Post(() => { + try + { Metar = e.Metar.RawMetar?.ToUpperInvariant(); Altimeter = e.Metar.Pressure?.Value?.ActualUnit == Value.Unit.HectoPascal ? "Q" + e.Metar.Pressure?.Value?.ActualValue.ToString("0000") : "A" + e.Metar.Pressure?.Value?.ActualValue.ToString("0000"); Wind = e.Metar.SurfaceWind?.RawValue; ObservationTime = e.Metar.Time.Replace(":", ""); propertyUpdates.SetResult(); + } + catch (Exception ex) + { + propertyUpdates.SetException(ex); + } }); -await propertyUpdates.Task; +try +{ + await propertyUpdates.Task; +} +catch (Exception ex) +{ + Log.Error(ex, "Failed to update METAR properties"); + throw; +}
1489-1493: Fix potential memory leak in voice server connection.The voice server connection update is not awaited, which could lead to orphaned tasks if cancellation occurs.
Apply this diff to fix the potential memory leak:
if (voiceAtis.AudioBytes != null && _networkConnection != null) { var dto = AtisBotUtils.AddBotRequest(voiceAtis.AudioBytes, _atisStation.Frequency, _atisStationAirport.Latitude, _atisStationAirport.Longitude, 100); - _voiceServerConnection?.AddOrUpdateBot(_networkConnection.Callsign, dto, - _cancellationToken.Token); + await _voiceServerConnection?.AddOrUpdateBot(_networkConnection.Callsign, dto, + _cancellationToken.Token)!; }
🧹 Nitpick comments (1)
vATIS.Desktop/Ui/ViewModels/AtisStationViewModel.cs (1)
1157-1164: Improve error handling in task continuation.The current error handling joins exception messages with commas, which could be hard to read and loses valuable stack trace information.
Apply this diff to improve error handling:
if (t.IsFaulted) { - ErrorMessage = string.Join(",", - t.Exception.InnerExceptions.Select(exception => exception.Message)); + var errors = t.Exception.InnerExceptions + .Select(ex => $"{ex.GetType().Name}: {ex.Message}") + .ToList(); + ErrorMessage = string.Join(Environment.NewLine, errors); + Log.Error(t.Exception, "Failed to update voice ATIS"); _networkConnection?.Disconnect(); NativeAudio.EmitSound(SoundType.Error); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
vATIS.Desktop/Ui/ViewModels/AtisStationViewModel.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build (macos-latest)
- GitHub Check: Analyze (cpp)
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze (csharp)
- GitHub Check: build (ubuntu-latest)
Summary by CodeRabbit