frontend: video-manager: Add block list support for MCM sources#3837
Open
joaoantoniocardoso wants to merge 1 commit intobluerobotics:1.4from
Open
frontend: video-manager: Add block list support for MCM sources#3837joaoantoniocardoso wants to merge 1 commit intobluerobotics:1.4from
joaoantoniocardoso wants to merge 1 commit intobluerobotics:1.4from
Conversation
Reviewer's GuideAdds UI, state handling, and store actions to support blocking/unblocking MCM video sources, updating status indicators, controls, and thumbnails behavior in the video manager frontend. Sequence diagram for blocking and unblocking a video source in the video managersequenceDiagram
actor User
participant VideoDeviceComponent
participant VideoStore
participant BackendAPI
User->>VideoDeviceComponent: Toggle block source switch
VideoDeviceComponent->>VideoDeviceComponent: toggleBlocked()
alt Device is currently unblocked
VideoDeviceComponent->>VideoStore: blockSource(device.source)
VideoStore->>BackendAPI: POST API_URL/block_source?source_string=source
alt Block request succeeds
BackendAPI-->>VideoStore: 200 OK
VideoStore->>VideoStore: fetchDevices()
VideoStore->>VideoStore: fetchStreams()
VideoStore-->>VideoDeviceComponent: Updated devices and streams state
VideoDeviceComponent->>VideoDeviceComponent: Recompute status_color, has_healthy_streams, thumbnails_disabled
else Block request fails
BackendAPI-->>VideoStore: Error
VideoStore->>VideoStore: notifier.pushError(VIDEO_SOURCE_BLOCK_FAIL)
end
else Device is currently blocked
VideoDeviceComponent->>VideoStore: unblockSource(device.source)
VideoStore->>BackendAPI: POST API_URL/unblock_source?source_string=source
alt Unblock request succeeds
BackendAPI-->>VideoStore: 200 OK
VideoStore->>VideoStore: fetchDevices()
VideoStore->>VideoStore: fetchStreams()
VideoStore-->>VideoDeviceComponent: Updated devices and streams state
VideoDeviceComponent->>VideoDeviceComponent: Recompute status_color, has_healthy_streams, thumbnails_disabled
else Unblock request fails
BackendAPI-->>VideoStore: Error
VideoStore->>VideoStore: notifier.pushError(VIDEO_SOURCE_UNBLOCK_FAIL)
end
end
User->>VideoDeviceComponent: View device card
VideoDeviceComponent->>VideoDeviceComponent: Render tooltip, Add stream button, thumbnail based on device.blocked and streams
Class diagram for updated video manager blocking supportclassDiagram
class Device {
string source
Format[] formats
Control[] controls
bool blocked
}
class VideoStore {
string API_URL
blockSource(source string) Promise~void~
unblockSource(source string) Promise~void~
fetchDevices() Promise~void~
fetchStreams() Promise~void~
resetSettings() Promise~void~
}
class VideoDeviceComponent {
+Device device
+bool show_controls_dialog
+bool show_stream_creation_dialog
+bool are_video_streams_available
+has_configs() bool
+has_healthy_streams() bool
+thumbnails_disabled() bool
+is_pirate_mode() bool
+status_color() string
+openControlsDialog() void
+openStreamCreationDialog() void
+toggleBlocked() Promise~void~
+createNewStream(stream CreatedStream) Promise~void~
}
class Settings {
bool is_pirate_mode
}
class VideoThumbnail {
+string source
+int width
+bool register
+bool disabled
}
VideoDeviceComponent --> Device : uses
VideoDeviceComponent --> VideoStore : dispatches_actions
VideoDeviceComponent --> Settings : reads
VideoDeviceComponent --> VideoThumbnail : renders
VideoStore --> Device : fetches
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Integrate the block list API from mavlink-camera-manager PR bluerobotics#561. Pirate-mode users can toggle source blocking via a switch; all users see the blocked state reflected in the status indicator and tooltip.
joaoantoniocardoso
force-pushed
the
1.4-add-block-list-mcm-sources
branch
from
9d98fe9 to
188055b
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In the
blockSourceandunblockSourceactions, you're mixingasync/awaitwith.then/.catch; consider refactoring to a singletry/catchwithawaitand explicitly awaitingfetchDevices()/fetchStreams()(e.g., viaawait Promise.all([...])) so errors propagate cleanly and the UI refresh is deterministic. - The
toggleBlockedmethod does not provide any loading/disabled state for thev-switch, so rapid toggling could trigger overlappingblockSource/unblockSourcerequests; consider tracking a per-device busy state to disable the switch while an update is in flight. - The
thumbnails_disabledcomputed currently disables thumbnails if any stream hasdisable_thumbnails === true; if the intent is to only disable when all associated streams request it, consider adjusting the predicate (e.g., using.every) to better match the desired behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `blockSource` and `unblockSource` actions, you're mixing `async/await` with `.then/.catch`; consider refactoring to a single `try/catch` with `await` and explicitly awaiting `fetchDevices()`/`fetchStreams()` (e.g., via `await Promise.all([...])`) so errors propagate cleanly and the UI refresh is deterministic.
- The `toggleBlocked` method does not provide any loading/disabled state for the `v-switch`, so rapid toggling could trigger overlapping `blockSource`/`unblockSource` requests; consider tracking a per-device busy state to disable the switch while an update is in flight.
- The `thumbnails_disabled` computed currently disables thumbnails if any stream has `disable_thumbnails === true`; if the intent is to only disable when all associated streams request it, consider adjusting the predicate (e.g., using `.every`) to better match the desired behavior.
## Individual Comments
### Comment 1
<location path="core/frontend/src/components/video-manager/VideoDevice.vue" line_range="55-62" />
<code_context>
<v-icon>mdi-plus</v-icon>
Add stream
</v-btn>
+ <v-switch
+ v-if="is_pirate_mode"
+ :input-value="device.blocked"
+ dense
+ hide-details
+ class="mt-2"
+ label="Block source"
+ @change="toggleBlocked"
+ />
</div>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider handling in-flight toggle state to avoid repeated block/unblock requests and confusing UI feedback.
The `v-switch` stays enabled and `toggleBlocked` doesn’t prevent overlapping requests, so rapid toggling can fire multiple `blockSource`/`unblockSource` calls while `device.blocked` is still stale from props. This can lead to a racy final state and UI lag. Track a local `blockingInProgress` flag to temporarily disable the switch (or ignore further changes) while the request is in flight, and only re-enable it once the updated state is loaded.
Suggested implementation:
```
<v-switch
v-if="is_pirate_mode"
:input-value="device.blocked"
dense
hide-details
class="mt-2"
label="Block source"
:disabled="blockingInProgress"
@change="toggleBlocked"
/>
```
Because I can only see the template portion of this component, a few more changes are required in the `<script>` section of `VideoDevice.vue`:
1. **Track in-flight state in local data**
In the component’s `data()` function, add the `blockingInProgress` flag:
```js
data () {
return {
// ...existing state
blockingInProgress: false,
};
}
```
2. **Update `toggleBlocked` to guard against overlapping requests**
Locate the existing `toggleBlocked` method and update it roughly as follows (adjust to match your current implementation and APIs):
```js
methods: {
async toggleBlocked (nextValue) {
// Prevent overlapping requests
if (this.blockingInProgress) {
return;
}
this.blockingInProgress = true;
try {
if (nextValue) {
// user turned the switch ON → block source
await this.blockSource(this.device.id); // or whatever call you are currently using
} else {
// user turned the switch OFF → unblock source
await this.unblockSource(this.device.id);
}
// Option A: if you manage the state locally, update it here:
// this.device.blocked = nextValue;
//
// Option B (recommended if `device` comes from Vuex/parent props):
// rely on store/parent to refresh `device.blocked` after the API call.
} finally {
this.blockingInProgress = false;
}
},
}
```
3. **Ensure state refresh after the API call**
If `device.blocked` is driven by Vuex or a parent component, make sure the block/unblock actions trigger a refresh of the device data so the prop reflects the final state. If you already have such a refresh (e.g. reloading the device list after the call), no extra work is needed.
These changes ensure that rapid toggling does not send overlapping requests and the `v-switch` is visually disabled while a block/unblock operation is in flight, matching your review comment.
</issue_to_address>
### Comment 2
<location path="core/frontend/src/store/video.ts" line_range="222-228" />
<code_context>
}
+ @Action
+ async blockSource(source: string): Promise<void> {
+ await back_axios({
+ method: 'post',
+ url: `${this.API_URL}/block_source`,
+ timeout: 10000,
+ params: { source_string: source },
+ })
+ .then(() => {
+ this.fetchDevices()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid mixing `async/await` with `.then/.catch` and ensure `fetchDevices/Streams` errors are handled.
In `blockSource`/`unblockSource` you `await back_axios(...)` and then chain `.then/.catch`. That means:
- Errors from `fetchDevices()` / `fetchStreams()` aren’t caught, since their promises aren’t awaited.
- Mixing `async/await` with `.then/.catch` makes the flow harder to follow.
Use a single `try/catch` with `await` for all async steps, e.g.:
```ts
@Action
async blockSource(source: string): Promise<void> {
try {
await back_axios({
method: 'post',
url: `${this.API_URL}/block_source`,
timeout: 10000,
params: { source_string: source },
})
await Promise.all([this.fetchDevices(), this.fetchStreams()])
} catch (error) {
const message = `Could not block video source: ${error.message}.`
notifier.pushError('VIDEO_SOURCE_BLOCK_FAIL', message)
}
}
```
Same pattern for `unblockSource` so all async work shares one clear error path.
Suggested implementation:
```typescript
@Action
async blockSource(source: string): Promise<void> {
try {
await back_axios({
method: 'post',
url: `${this.API_URL}/block_source`,
timeout: 10000,
params: { source_string: source },
})
await Promise.all([this.fetchDevices(), this.fetchStreams()])
} catch (error: any) {
const message = `Could not block video source: ${error.message}.`
notifier.pushError('VIDEO_SOURCE_BLOCK_FAIL', message)
}
}
```
You should apply the same pattern to `unblockSource`:
1. Wrap its body in a `try/catch`.
2. `await` the `back_axios` call to the `/unblock_source` endpoint.
3. `await Promise.all([this.fetchDevices(), this.fetchStreams()])` after the request.
4. In `catch`, construct an appropriate error message and push it via `notifier.pushError` (likely with a key such as `VIDEO_SOURCE_UNBLOCK_FAIL` to match your existing conventions).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+55
to
+62
| <v-switch | ||
| v-if="is_pirate_mode" | ||
| :input-value="device.blocked" | ||
| dense | ||
| hide-details | ||
| class="mt-2" | ||
| label="Block source" | ||
| @change="toggleBlocked" |
There was a problem hiding this comment.
suggestion (bug_risk): Consider handling in-flight toggle state to avoid repeated block/unblock requests and confusing UI feedback.
The v-switch stays enabled and toggleBlocked doesn’t prevent overlapping requests, so rapid toggling can fire multiple blockSource/unblockSource calls while device.blocked is still stale from props. This can lead to a racy final state and UI lag. Track a local blockingInProgress flag to temporarily disable the switch (or ignore further changes) while the request is in flight, and only re-enable it once the updated state is loaded.
Suggested implementation:
<v-switch
v-if="is_pirate_mode"
:input-value="device.blocked"
dense
hide-details
class="mt-2"
label="Block source"
:disabled="blockingInProgress"
@change="toggleBlocked"
/>
Because I can only see the template portion of this component, a few more changes are required in the <script> section of VideoDevice.vue:
-
Track in-flight state in local data
In the component’sdata()function, add theblockingInProgressflag:data () { return { // ...existing state blockingInProgress: false, }; }
-
Update
toggleBlockedto guard against overlapping requests
Locate the existingtoggleBlockedmethod and update it roughly as follows (adjust to match your current implementation and APIs):methods: { async toggleBlocked (nextValue) { // Prevent overlapping requests if (this.blockingInProgress) { return; } this.blockingInProgress = true; try { if (nextValue) { // user turned the switch ON → block source await this.blockSource(this.device.id); // or whatever call you are currently using } else { // user turned the switch OFF → unblock source await this.unblockSource(this.device.id); } // Option A: if you manage the state locally, update it here: // this.device.blocked = nextValue; // // Option B (recommended if `device` comes from Vuex/parent props): // rely on store/parent to refresh `device.blocked` after the API call. } finally { this.blockingInProgress = false; } }, }
-
Ensure state refresh after the API call
Ifdevice.blockedis driven by Vuex or a parent component, make sure the block/unblock actions trigger a refresh of the device data so the prop reflects the final state. If you already have such a refresh (e.g. reloading the device list after the call), no extra work is needed.
These changes ensure that rapid toggling does not send overlapping requests and the v-switch is visually disabled while a block/unblock operation is in flight, matching your review comment.
Comment on lines
+222
to
+228
| async blockSource(source: string): Promise<void> { | ||
| await back_axios({ | ||
| method: 'post', | ||
| url: `${this.API_URL}/block_source`, | ||
| timeout: 10000, | ||
| params: { source_string: source }, | ||
| }) |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid mixing async/await with .then/.catch and ensure fetchDevices/Streams errors are handled.
In blockSource/unblockSource you await back_axios(...) and then chain .then/.catch. That means:
- Errors from
fetchDevices()/fetchStreams()aren’t caught, since their promises aren’t awaited. - Mixing
async/awaitwith.then/.catchmakes the flow harder to follow.
Use a single try/catch with await for all async steps, e.g.:
@Action
async blockSource(source: string): Promise<void> {
try {
await back_axios({
method: 'post',
url: `${this.API_URL}/block_source`,
timeout: 10000,
params: { source_string: source },
})
await Promise.all([this.fetchDevices(), this.fetchStreams()])
} catch (error) {
const message = `Could not block video source: ${error.message}.`
notifier.pushError('VIDEO_SOURCE_BLOCK_FAIL', message)
}
}Same pattern for unblockSource so all async work shares one clear error path.
Suggested implementation:
@Action
async blockSource(source: string): Promise<void> {
try {
await back_axios({
method: 'post',
url: `${this.API_URL}/block_source`,
timeout: 10000,
params: { source_string: source },
})
await Promise.all([this.fetchDevices(), this.fetchStreams()])
} catch (error: any) {
const message = `Could not block video source: ${error.message}.`
notifier.pushError('VIDEO_SOURCE_BLOCK_FAIL', message)
}
}You should apply the same pattern to unblockSource:
- Wrap its body in a
try/catch. awaittheback_axioscall to the/unblock_sourceendpoint.await Promise.all([this.fetchDevices(), this.fetchStreams()])after the request.- In
catch, construct an appropriate error message and push it vianotifier.pushError(likely with a key such asVIDEO_SOURCE_UNBLOCK_FAILto match your existing conventions).
patrickelectric
approved these changes
Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of 3662db48 to 1.4.
Adds block list support for MCM sources in the video manager frontend.
Summary by Sourcery
Add block list support and related UI updates for video sources in the video manager frontend.
New Features:
Enhancements: