bluerobotics / cockpit

An intuitive and customizable cross-platform ground control station for remote vehicles of all types.
https://blueos.cloud/cockpit/docs
Other
72 stars 22 forks source link

Add input to rename internal video streams #1359

Closed ArturoManzoli closed 1 month ago

ArturoManzoli commented 2 months ago

Now user can double click the stream name or press the 'pencil' icon to change its internal name:

https://github.com/user-attachments/assets/524e7c78-e32e-4c39-8766-c9d892d98153

Closes #1336

ArturoManzoli commented 1 month ago

When the internal stream name is changed, are all the widgets that are connected to it losing their connection?

Yes, they are. I think is alright to automate that.

rafaellehmkuhl commented 1 month ago

When the internal stream name is changed, are all the widgets that are connected to it losing their connection?

Yes, they are. I think is alright to automate that.

There are probably two options here:

The first option is the easiest to implement, but it's much more likely to incur in bugs. The second is probably the right approach, unlikely to create any bug situations, as it removes the need for any side-effects, but involves a little more work.

ArturoManzoli commented 1 month ago

The first option is the easiest to implement, but it's much more likely to incur in bugs. The second is probably the right approach, unlikely to create any bug situations, as it removes the need for any side-effects, but involves a little more work.

I went for option 1, but using a very controlled approach

rafaellehmkuhl commented 1 month ago

The first option is the easiest to implement, but it's much more likely to incur in bugs. The second is probably the right approach, unlikely to create any bug situations, as it removes the need for any side-effects, but involves a little more work.

I went for option 1, but using a very controlled approach

Just to make sure, if the widgets were assigned to one stream, and this stream had its internal name changed, what is going to happen? Is it creating some error or is it auto-connecting to the first one available?

ArturoManzoli commented 1 month ago

The first option is the easiest to implement, but it's much more likely to incur in bugs. The second is probably the right approach, unlikely to create any bug situations, as it removes the need for any side-effects, but involves a little more work.

I went for option 1, but using a very controlled approach

Just to make sure, if the widgets were assigned to one stream, and this stream had its internal name changed, what is going to happen? Is it creating some error or is it auto-connecting to the first one available?

After a stream is renamed, both the player and mini-recorder will check if the renamed stream is the one they are using. If it is, they will retrieve the new name from the video store and replace the old (renamed) one. If not, nothing happens.

ArturoManzoli commented 1 month ago

It's all working fine for me!

I would ask you only to test it as much as possible before the next release. I will do the same here.

My main concern is if people rename streams and the widgets don't get it, specially the video recorder, so a user clicks the record button expecting things to be recorded and they are not. I didn't find any signs for this situation in the code thought.

I agree with your concerns. I'll test for that also