feat: Add the logic for switching cameras by USB groups.#500
Merged
Conversation
Reviewer's GuideAdds USB camera grouping support controlled by configuration, and integrates group-based camera switching logic into videowidget while extending DataManager and main startup to manage the feature flag. Sequence diagram for USB group-based camera switching in videowidget::onChangeDevsequenceDiagram
actor User
participant videowidget
participant DataManager
participant V4L2
User->>videowidget: onChangeDev()
videowidget->>V4L2: get_device_list()
V4L2-->>videowidget: devlist
videowidget->>DataManager: isEnableUsbGroup()
DataManager-->>videowidget: enableUsbGroup
alt [devlist is nullptr]
videowidget-->>User: qWarning get device list FAILED
else [enableUsbGroup is false]
alt [devlist->num_devices == 2]
videowidget->>videowidget: switchCamera(other device)
else [devlist->num_devices != 2]
videowidget->>videowidget: switchCamera(next device or first)
videowidget->>DataManager: setdevStatus(NOCAM) when num_devices == 0
end
else [enableUsbGroup is true]
videowidget->>videowidget: getUSBCameraGroup(devlist, vGroupData)
videowidget-->>videowidget: groupNum
alt [groupNum == 1]
videowidget->>videowidget: switchCamera using devlist
else [groupNum == 2]
videowidget->>videowidget: switchCamera using vGroupData first device per group
else [groupNum > 2]
videowidget->>videowidget: switchCamera cycle groups via vGroupData
videowidget->>DataManager: setdevStatus(NOCAM) when devlist->num_devices == 0
end
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 2 issues, and left some high level feedback:
- Consider clearing
vGroupDataat the beginning ofgetUSBCameraGroupto avoid unintentionally appending to existing data if the same container is reused across calls. - The camera-switching logic in
onChangeDevnow has three separate code paths (no USB group, 2 groups, >2 groups) with very similar iteration/switch patterns; refactoring the common parts into a helper would reduce duplication and make the behavior easier to reason about. - In the USB group branches you always use
second[0]of each group and ignore other devices within the group; if this is intentional it might be worth enforcing or asserting that each group only has one device, otherwise consider how multiple cameras per group should be handled.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider clearing `vGroupData` at the beginning of `getUSBCameraGroup` to avoid unintentionally appending to existing data if the same container is reused across calls.
- The camera-switching logic in `onChangeDev` now has three separate code paths (no USB group, 2 groups, >2 groups) with very similar iteration/switch patterns; refactoring the common parts into a helper would reduce duplication and make the behavior easier to reason about.
- In the USB group branches you always use `second[0]` of each group and ignore other devices within the group; if this is intentional it might be worth enforcing or asserting that each group only has one device, otherwise consider how multiple cameras per group should be handled.
## Individual Comments
### Comment 1
<location path="src/src/videowidget.cpp" line_range="1252-1253" />
<code_context>
+ }
+
+ // USB摄像头分组相关逻辑来自xiwo分支，是否开启由DConfig控制
+ int groupNum = 1;
+ QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> vGroupData;
+ // 如果未启用USB摄像头分组，则分组数默认为1，保持原有逻辑；
+ // 如果启用USB摄像头分组，则实际获取USB分组情况，根据分组结果进行处理；
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider clearing vGroupData inside getUSBCameraGroup to avoid accidental reuse bugs.
The function’s correctness currently depends on callers ensuring `vGroupData` is empty. If a future caller passes a non-empty vector, the function will append to existing data, which can break the grouping logic. Clearing `vGroupData` at the start of `getUSBCameraGroup` would make the helper robust and independent of caller assumptions.
Suggested implementation:
```cpp
int VideoWidget::getUSBCameraGroup(usb_dev_list_t *devlist, QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> &vGroupData)
{
// 确保每次调用从空的分组数据开始，避免在非空 vGroupData 上追加导致分组逻辑异常
vGroupData.clear();
```
If the function signature differs (e.g. not a member of `VideoWidget`, or parameter types/names vary), adjust the `SEARCH` line to match the exact function declaration of `getUSBCameraGroup` and keep the `vGroupData.clear();` as the first statement in the function body. No changes are needed at the call site shown in your snippet, since clearing is handled internally by the helper.
</issue_to_address>
### Comment 2
<location path="src/src/videowidget.cpp" line_range="1263" />
<code_context>
+ groupNum = getUSBCameraGroup(devlist, vGroupData);
+ qInfo() << __func__ << "groupNum:" << groupNum;
+ }
+ if (groupNum == 1) {
+ if (devlist->num_devices == 2) {
+ for (int i = 0 ; i < devlist->num_devices; i++) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider introducing a shared CameraEntry abstraction and a single helper for camera switching so onChangeDev() handles grouped and non-grouped devices through one unified flow instead of multiple duplicated branches.
You can significantly reduce the branching/duplication in `onChangeDev()` by introducing a small, common abstraction over “camera entries” and a reusable helper for the switching logic. This keeps the USB grouping feature intact while simplifying the flow.
### 1. Unify grouped / non‑grouped access via `CameraEntry`
Instead of indexing into `devlist` and `vGroupData` separately inside `onChangeDev()`, build a flat list of “switchable cameras” and use that everywhere:
```cpp
struct CameraEntry {
const char *device;
const char *name;
};
static QVector<CameraEntry> buildCameraListFromDevlist(v4l2_device_list_t *devlist)
{
QVector<CameraEntry> cameras;
cameras.reserve(devlist->num_devices);
for (int i = 0; i < devlist->num_devices; ++i) {
cameras.append({
devlist->list_devices[i].device,
devlist->list_devices[i].name
});
}
return cameras;
}
static QVector<CameraEntry> buildCameraListFromGroups(
const QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> &vGroupData)
{
QVector<CameraEntry> cameras;
cameras.reserve(vGroupData.count());
for (int i = 0; i < vGroupData.count(); ++i) {
auto *dev = vGroupData[i].second[0];
cameras.append({ dev->device, dev->name });
}
return cameras;
}
```
### 2. Extract the switching logic into a single helper
The “toggle when there are exactly two” and “rotate/wrap otherwise” behavior is identical for grouped and non‑grouped flows. You can express it once:
```cpp
static void switchToNextCamera(
const QVector<CameraEntry> &cameras,
const QString ¤tDevice,
bool isToggleMode, // true for "exactly two" behavior
videowidget *vw) // or capture `this` in a lambda
{
if (cameras.isEmpty()) {
DataManager::instance()->setdevStatus(NOCAM);
vw->showNocam();
return;
}
if (isToggleMode && cameras.size() == 2) {
// toggle: pick the other device
for (const auto &cam : cameras) {
if (currentDevice != cam.device) {
if (E_OK == vw->switchCamera(cam.device, cam.name))
return;
}
}
return;
}
// rotation / wrap
if (currentDevice.isEmpty()) {
vw->switchCamera(cameras[0].device, cameras[0].name);
return;
}
for (int i = 0; i < cameras.size(); ++i) {
if (currentDevice == cameras[i].device) {
const int nextIdx = (i == cameras.size() - 1) ? 0 : i + 1;
vw->switchCamera(cameras[nextIdx].device, cameras[nextIdx].name);
return;
}
}
}
```
### 3. Simplify `onChangeDev()` to use the helpers
The existing branching on `groupNum==1` / `groupNum==2` and `num_devices==2` can be reduced to computing the camera list and a mode flag:
```cpp
void videowidget::onChangeDev()
{
while (m_imgPrcThread->isRunning());
QString str;
if (devicehandler != nullptr) {
qDebug() << "Closing v4l2 device handler";
str = QString(devicehandler->videodevice);
close_v4l2_device_handler();
}
v4l2_device_list_t *devlist = get_device_list();
if (!devlist) {
qWarning() << "get device list FAILED";
return;
}
int groupNum = 1;
QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> vGroupData;
if (DataManager::instance()->isEnableUsbGroup()) {
groupNum = getUSBCameraGroup(devlist, vGroupData);
qInfo() << __func__ << "groupNum:" << groupNum;
}
QVector<CameraEntry> cameras;
bool isToggleMode = false;
if (groupNum == 1) {
cameras = buildCameraListFromDevlist(devlist);
isToggleMode = (devlist->num_devices == 2);
} else {
cameras = buildCameraListFromGroups(vGroupData);
isToggleMode = (groupNum == 2);
}
switchToNextCamera(cameras, str, isToggleMode, this);
qDebug() << "Camera device change complete";
}
```
This:
- Removes duplicated loops and manual indexing into `devlist` / `vGroupData`.
- Encapsulates the “toggle when there are exactly two” vs “rotate/wrap” behavior.
- Keeps all existing functionality, including USB grouping and the `groupNum` semantics.
### 4. (Optional) Clarify grouping type for `getUSBCameraGroup`
If you later decide to clean up `getUSBCameraGroup`, a simple domain type will further reduce cognitive load:
```cpp
struct CameraGroup {
QString location;
QVector<v4l2_dev_sys_data_t *> devices;
};
int videowidget::getUSBCameraGroup(
v4l2_device_list_t *devlist,
QVector<CameraGroup> &groups)
{
if (!devlist) {
qWarning() << __func__ << "devlist is NULL!";
return 0;
}
for (int i = 0; i < devlist->num_devices; ++i) {
QString location = QString(devlist->list_devices[i].location);
int j = 0;
for (; j < groups.size(); ++j) {
if (location == groups[j].location)
break;
}
if (j == groups.size()) {
CameraGroup group;
group.location = location;
group.devices.append(&devlist->list_devices[i]);
groups.append(group);
} else {
groups[j].devices.append(&devlist->list_devices[i]);
}
}
return groups.size();
}
```
You can then adapt `buildCameraListFromGroups` to use `QVector<CameraGroup>` instead of `QVector<QPair<...>>` without changing behavior.
</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
+1252
to
+1253
| int groupNum = 1; | ||
| QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> vGroupData; |
There was a problem hiding this comment.
suggestion (bug_risk): Consider clearing vGroupData inside getUSBCameraGroup to avoid accidental reuse bugs.
The function’s correctness currently depends on callers ensuring vGroupData is empty. If a future caller passes a non-empty vector, the function will append to existing data, which can break the grouping logic. Clearing vGroupData at the start of getUSBCameraGroup would make the helper robust and independent of caller assumptions.
Suggested implementation:
int VideoWidget::getUSBCameraGroup(usb_dev_list_t *devlist, QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> &vGroupData)
{
// 确保每次调用从空的分组数据开始，避免在非空 vGroupData 上追加导致分组逻辑异常
vGroupData.clear();
If the function signature differs (e.g. not a member of VideoWidget, or parameter types/names vary), adjust the SEARCH line to match the exact function declaration of getUSBCameraGroup and keep the vGroupData.clear(); as the first statement in the function body. No changes are needed at the call site shown in your snippet, since clearing is handled internally by the helper.
| groupNum = getUSBCameraGroup(devlist, vGroupData); | ||
| qInfo() << __func__ << "groupNum:" << groupNum; | ||
| } | ||
| if (groupNum == 1) { |
There was a problem hiding this comment.
issue (complexity): Consider introducing a shared CameraEntry abstraction and a single helper for camera switching so onChangeDev() handles grouped and non-grouped devices through one unified flow instead of multiple duplicated branches.
You can significantly reduce the branching/duplication in onChangeDev() by introducing a small, common abstraction over “camera entries” and a reusable helper for the switching logic. This keeps the USB grouping feature intact while simplifying the flow.
1. Unify grouped / non‑grouped access via CameraEntry
Instead of indexing into devlist and vGroupData separately inside onChangeDev(), build a flat list of “switchable cameras” and use that everywhere:
struct CameraEntry {
const char *device;
const char *name;
};
static QVector<CameraEntry> buildCameraListFromDevlist(v4l2_device_list_t *devlist)
{
QVector<CameraEntry> cameras;
cameras.reserve(devlist->num_devices);
for (int i = 0; i < devlist->num_devices; ++i) {
cameras.append({
devlist->list_devices[i].device,
devlist->list_devices[i].name
});
}
return cameras;
}
static QVector<CameraEntry> buildCameraListFromGroups(
const QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> &vGroupData)
{
QVector<CameraEntry> cameras;
cameras.reserve(vGroupData.count());
for (int i = 0; i < vGroupData.count(); ++i) {
auto *dev = vGroupData[i].second[0];
cameras.append({ dev->device, dev->name });
}
return cameras;
}2. Extract the switching logic into a single helper
The “toggle when there are exactly two” and “rotate/wrap otherwise” behavior is identical for grouped and non‑grouped flows. You can express it once:
static void switchToNextCamera(
const QVector<CameraEntry> &cameras,
const QString ¤tDevice,
bool isToggleMode, // true for "exactly two" behavior
videowidget *vw) // or capture `this` in a lambda
{
if (cameras.isEmpty()) {
DataManager::instance()->setdevStatus(NOCAM);
vw->showNocam();
return;
}
if (isToggleMode && cameras.size() == 2) {
// toggle: pick the other device
for (const auto &cam : cameras) {
if (currentDevice != cam.device) {
if (E_OK == vw->switchCamera(cam.device, cam.name))
return;
}
}
return;
}
// rotation / wrap
if (currentDevice.isEmpty()) {
vw->switchCamera(cameras[0].device, cameras[0].name);
return;
}
for (int i = 0; i < cameras.size(); ++i) {
if (currentDevice == cameras[i].device) {
const int nextIdx = (i == cameras.size() - 1) ? 0 : i + 1;
vw->switchCamera(cameras[nextIdx].device, cameras[nextIdx].name);
return;
}
}
}3. Simplify onChangeDev() to use the helpers
The existing branching on groupNum==1 / groupNum==2 and num_devices==2 can be reduced to computing the camera list and a mode flag:
void videowidget::onChangeDev()
{
while (m_imgPrcThread->isRunning());
QString str;
if (devicehandler != nullptr) {
qDebug() << "Closing v4l2 device handler";
str = QString(devicehandler->videodevice);
close_v4l2_device_handler();
}
v4l2_device_list_t *devlist = get_device_list();
if (!devlist) {
qWarning() << "get device list FAILED";
return;
}
int groupNum = 1;
QVector<QPair<QString, QVector<v4l2_dev_sys_data_t *>>> vGroupData;
if (DataManager::instance()->isEnableUsbGroup()) {
groupNum = getUSBCameraGroup(devlist, vGroupData);
qInfo() << __func__ << "groupNum:" << groupNum;
}
QVector<CameraEntry> cameras;
bool isToggleMode = false;
if (groupNum == 1) {
cameras = buildCameraListFromDevlist(devlist);
isToggleMode = (devlist->num_devices == 2);
} else {
cameras = buildCameraListFromGroups(vGroupData);
isToggleMode = (groupNum == 2);
}
switchToNextCamera(cameras, str, isToggleMode, this);
qDebug() << "Camera device change complete";
}This:
- Removes duplicated loops and manual indexing into
devlist/vGroupData. - Encapsulates the “toggle when there are exactly two” vs “rotate/wrap” behavior.
- Keeps all existing functionality, including USB grouping and the
groupNumsemantics.
4. (Optional) Clarify grouping type for getUSBCameraGroup
If you later decide to clean up getUSBCameraGroup, a simple domain type will further reduce cognitive load:
struct CameraGroup {
QString location;
QVector<v4l2_dev_sys_data_t *> devices;
};
int videowidget::getUSBCameraGroup(
v4l2_device_list_t *devlist,
QVector<CameraGroup> &groups)
{
if (!devlist) {
qWarning() << __func__ << "devlist is NULL!";
return 0;
}
for (int i = 0; i < devlist->num_devices; ++i) {
QString location = QString(devlist->list_devices[i].location);
int j = 0;
for (; j < groups.size(); ++j) {
if (location == groups[j].location)
break;
}
if (j == groups.size()) {
CameraGroup group;
group.location = location;
group.devices.append(&devlist->list_devices[i]);
groups.append(group);
} else {
groups[j].devices.append(&devlist->list_devices[i]);
}
}
return groups.size();
}You can then adapt buildCameraListFromGroups to use QVector<CameraGroup> instead of QVector<QPair<...>> without changing behavior.
Contributor
Author
|
/retest |
1 similar comment
Contributor
Author
|
/retest |
add-uos
force-pushed
the
master
branch
2 times, most recently
from
July 21, 2026 06:30
eb23c98 to
15b66d8
Compare
Contributor
|
/retest |
Add the logic for switching cameras by USB groups. 增加按USB分组切换摄像头的逻辑。 代码来自xiwo分支。 pick from: 6934da1
deepin pr auto review★ 总体评分：100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 在 videowidget.cpp 的 onChangeDev 函数中
if (groupNum == 1) {
if (devlist->num_devices == 2) {
for (int i = 0 ; i < devlist->num_devices; i++) {
QString curDev = QString(devlist->list_devices[i].device);
if (str != curDev) {
if (E_OK == switchCamera(devlist->list_devices[i].device, devlist->list_devices[i].name)) {
break;
}
}
}
}
// ... 其他逻辑同理修改
} |
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
Author
|
/merge |
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 optional USB camera grouping support and update camera switching logic to respect configured groups when changing devices.
New Features:
Enhancements:
Chores: