wisniax / RoverControlApp

Rover Control Application
https://raptors.p.lodz.pl/
2 stars 4 forks source link

Crashes on master - appling settings #31

Open Wojtuz opened 1 week ago

Wojtuz commented 1 week ago

Need to look into it more to pinpoint what exactly causes this issue image

AlvaroBajceps commented 1 week ago

bb59ede44ae9ebae52c216b90e531820207d6892 brought this bug to the light. Exception occurs when Mqtt ConnectionState is changed to 'Closed'. ConnectionState change triggers sending new message to mqtt https://github.com/wisniax/RoverControlApp/blob/1d68f915e9b92f65994228cc62b246b6f5e17685/MVVM/Model/RoverCommunication.cs#L102-L106 In that exact moment the _managedMqttClient is null (Due to settings change causing restart of MqttNode, which is needed to establish new connection). https://github.com/wisniax/RoverControlApp/blob/1d68f915e9b92f65994228cc62b246b6f5e17685/MVVM/Model/MqttNode.cs#L116-L120 That is how we got here

AlvaroBajceps commented 1 week ago

I propose dirty-fix: Change code to do not send any message when MqttNode.ConnectionState is not 'Opened' or 'Faulted'(faulted mean connection is not established but message will be queued correctly). something like this:

https://github.com/wisniax/RoverControlApp/blob/1d68f915e9b92f65994228cc62b246b6f5e17685/MVVM/Model/RoverCommunication.cs#L73-L77

private async void OnMqttConnectionChanged(CommunicationState arg) 
{ 
if (arg != CommunicationState.Opened && arg != CommunicationState.Faulted) return; // dirty-fix
await RoverMovementVectorChanged(_pressedKeys.RoverMovement); 
await RoverCommunication_OnControlStatusChanged(GenerateRoverStatus(connection: arg)); 
} 

Proper-way to do this would be to provide caching mechanism to hold enqueued messages when _managedMqttClient is in an invalid state or restarting.