feat: Add the logic for switching cameras by USB groups.#499
Closed
add-uos wants to merge 1 commit into
Closed
Conversation
Add the logic for switching cameras by USB groups. 增加按USB分组切换摄像头的逻辑。 代码来自xiwo分支。 pick from: 6934da1
Reviewer's GuideAdds configuration-controlled USB camera grouping and updates camera switching logic to operate by groups when enabled, while preserving existing behavior when grouping is disabled or unavailable. Sequence diagram for camera switching with optional USB groupingsequenceDiagram
actor User
participant videowidget
participant DataManager
participant v4l2
User->>videowidget: onChangeDev()
videowidget->>v4l2: get_device_list()
v4l2-->>videowidget: devlist
videowidget->>videowidget: [devlist == nullptr]
videowidget-->>User: return
videowidget->>DataManager: isEnableUsbGroup()
DataManager-->>videowidget: enableUsbGroup
alt [enableUsbGroup == false]
videowidget->>videowidget: switchCamera(...) using devlist
else [enableUsbGroup == true]
videowidget->>videowidget: getUSBCameraGroup(devlist, vGroupData)
videowidget->>videowidget: [groupNum == 2]
videowidget->>videowidget: switchCamera(...) using first device in each group
else [groupNum != 2]
videowidget->>videowidget: switchCamera(...) cycling through groups
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 1 issue, and left some high level feedback:
- The camera switching logic for grouped and non-grouped USB devices is now quite duplicated and branch-heavy; consider extracting a small helper that encapsulates the "next camera" selection to reduce repetition and make the flow easier to follow.
- In the USB group handling (
getUSBCameraGroupand subsequent use), you always indexvGroupData[i].second[0]without checking that each group's device list is non-empty; adding guards for empty groups would make the code more robust against unexpected input. - The grouping structure
QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>>is a bit opaque and requires manual searching; even if you keep this type, wrapping it in a small struct or helper API for lookup and iteration could improve readability and reduce error-prone index arithmetic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The camera switching logic for grouped and non-grouped USB devices is now quite duplicated and branch-heavy; consider extracting a small helper that encapsulates the "next camera" selection to reduce repetition and make the flow easier to follow.
- In the USB group handling (`getUSBCameraGroup` and subsequent use), you always index `vGroupData[i].second[0]` without checking that each group's device list is non-empty; adding guards for empty groups would make the code more robust against unexpected input.
- The grouping structure `QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>>` is a bit opaque and requires manual searching; even if you keep this type, wrapping it in a small struct or helper API for lookup and iteration could improve readability and reduce error-prone index arithmetic.
## Individual Comments
### Comment 1
<location path="src/src/videowidget.cpp" line_range="1390-1399" />
<code_context>
+int videowidget::getUSBCameraGroup(v4l2_device_list_t *devlist, QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> &vGroupData)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** `getUSBCameraGroup` assumes `vGroupData` is empty and will append to existing content if reused.
Currently `vGroupData` is always appended to and never cleared. With the existing usage (`onChangeDev`) this works because a new empty vector is passed each time. But the signature allows for reuse with a non-empty `vGroupData`, which would cause repeated calls to accumulate stale groups and double-count entries. To avoid that, either clear `vGroupData` at the start of the function or explicitly enforce/document that it must be empty on entry (e.g., via an assertion).
</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
+1390
to
+1399
| int videowidget::getUSBCameraGroup(v4l2_device_list_t *devlist, QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> &vGroupData) | ||
| { | ||
| // 来自xiwo分支，根据location进行分组 | ||
| // 收到建议使用 QMap<QString, QVector<v4l2_dev_sys_data_t *>> 来存储分组数据，但我们担心影响现有代码逻辑， | ||
| // 所以暂时保留 QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> 来存储分组数据。 | ||
| if (devlist == nullptr) { | ||
| qWarning() << __func__ << "devlist is NULL!"; | ||
| return 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
suggestion (bug_risk): getUSBCameraGroup assumes vGroupData is empty and will append to existing content if reused.
Currently vGroupData is always appended to and never cleared. With the existing usage (onChangeDev) this works because a new empty vector is passed each time. But the signature allows for reuse with a non-empty vGroupData, which would cause repeated calls to accumulate stale groups and double-count entries. To avoid that, either clear vGroupData at the start of the function or explicitly enforce/document that it must be empty on entry (e.g., via an assertion).
deepin pr auto review★ 总体评分：100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 优化 getUSBCameraGroup 使用 QMap 提升可读性和效率，并在 onChangeDev 中增加安全检查
int videowidget::getUSBCameraGroup(v4l2_device_list_t *devlist, QMap<QString, QVector<v4l2_dev_sys_data_t *>> &groupData)
{
if (devlist == nullptr) {
qWarning() << __func__ << "devlist is NULL!";
return 0;
}
for (int i = 0; i < devlist->num_devices; i++) {
QString location = QString(devlist->list_devices[i].location);
groupData[location].append(&devlist->list_devices[i]);
}
return groupData.count();
}
// 在 onChangeDev 中使用时
// ...
if (!vGroupData.value(i).second.isEmpty()) {
const char *curDev = vGroupData.value(i).second.at(0)->device;
// ...
}
// ... |
lzwind
approved these changes
Jul 21, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, lzwind The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
|
/retest |
2 similar comments
Contributor
|
/retest |
Contributor
|
/retest |
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
4 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.
Add the logic for switching cameras by USB groups. 增加按USB分组切换摄像头的逻辑。
代码来自xiwo分支。
pick from: 6934da1
Summary by Sourcery
Add configurable support for switching cameras based on USB group information while preserving existing behavior when grouping is disabled.
New Features:
Enhancements: