feat: enable the configuration for device blacklist.#463
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom Mar 19, 2026
Merged
feat: enable the configuration for device blacklist.#463deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
Conversation
Reviewer's GuideImplements a device blacklist via DConfig and threads it through startup, device enumeration, and UI logic so that blacklisted or invalid camera devices are skipped and fall back to a valid/default device while also controlling when the camera-switch button is shown. Sequence diagram for startup and device blacklist applicationsequenceDiagram
participant main_cpp as main
participant DConfig as DConfig
participant DataManager as DataManager
participant VideoWidget as videowidget
participant V4L2 as v4l2core
main_cpp->>DConfig: create dconfig
DConfig-->>main_cpp: dconfig object
main_cpp->>DConfig: isValid()
DConfig-->>main_cpp: bool
main_cpp->>DConfig: keyList()
DConfig-->>main_cpp: keys
main_cpp->>DConfig: value(deviceBlacklist)
DConfig-->>main_cpp: QStringList blacklist
main_cpp->>DataManager: setDeviceBlacklist(blacklist)
main_cpp->>VideoWidget: construct videowidget
VideoWidget->>VideoWidget: delayInit()
VideoWidget->>V4L2: v4l2core_get_device_index(configDevice)
V4L2-->>VideoWidget: idx
alt config device empty or idx < 0
VideoWidget->>VideoWidget: getFirstValidDevice()
VideoWidget->>V4L2: get_device_list()
V4L2-->>VideoWidget: v4l2_device_list_t
loop each device
VideoWidget->>DataManager: isDeviceValid(vid,pid,name)
DataManager-->>VideoWidget: bool
end
VideoWidget->>V4L2: switchCamera(validDevice)
else config device found
VideoWidget->>V4L2: get_device_list()
V4L2-->>VideoWidget: v4l2_device_list_t
VideoWidget->>DataManager: isDeviceValid(vid,pid,name)
DataManager-->>VideoWidget: bool
alt device blacklisted
VideoWidget->>VideoWidget: getFirstValidDevice()
VideoWidget->>V4L2: switchCamera(validDevice)
else device valid
VideoWidget->>V4L2: switchCamera(configDevice)
end
end
Class diagram for new device blacklist handlingclassDiagram
class DataManager {
-static DataManager *m_dataManager
-QStringList m_deviceBlacklist
-bool m_isPreviewNoDelay
-bool m_enableUsbGroup
-bool m_enable8kPreview
+static DataManager* instance()
+bool isEnable8kPreview() const
+void setDeviceBlacklist(QStringList blacklist)
+bool isDeviceValid(QString vid, QString pid, QString name)
}
class videowidget {
+void delayInit()
+QString getSaveFilePrefix()
+QString getFirstValidDevice()
+MajorImageProcessingThread *m_imgPrcThread
+AudioProcessingThread *m_audPrcThread
}
class CMainWindow {
-bool m_bSwitchCameraShowEnable
+void setSelBtnShow()
}
class Camera {
-std::vector<QString> m_cameraDevList
+void refreshCamDevList()
+int parseConfig()
}
class v4l2_device_list_t {
+int num_devices
+v4l2_dev_sys_data_t *list_devices
}
class v4l2_dev_sys_data_t {
+int vendor
+int product
+int valid
+QString name
+QString device
}
class main_cpp {
+int main(int argc, char *argv[])
}
main_cpp --> DataManager : setDeviceBlacklist
videowidget --> DataManager : isDeviceValid
videowidget --> v4l2_device_list_t : get_device_list
videowidget --> v4l2_dev_sys_data_t : read vendor,product,valid,name,device
CMainWindow --> v4l2_device_list_t : get_device_list
CMainWindow --> v4l2_dev_sys_data_t : read/write valid, read vendor,product,name
CMainWindow --> DataManager : isDeviceValid
Camera --> v4l2_device_list_t : get_device_list
Camera --> v4l2_dev_sys_data_t : read valid,device
Camera --> DataManager : (indirect filtering via valid flag)
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:
- The logic for walking
v4l2_device_list_t, checkingvalid, and applyingDataManager::isDeviceValidis duplicated in several places (videowidget::delayInit,videowidget::getFirstValidDevice,CMainWindow::setSelBtnShow,Camera::refreshCamDevList); consider extracting a shared helper to produce a filtered list of valid devices so the blacklist behavior stays consistent and is easier to maintain. - In
CMainWindow::setSelBtnShow, whenvalidNum <= 1you return early without updatingm_bSwitchCameraShowEnableor hiding the switch button, which could leave a stale visible switch button if the number of valid devices drops at runtime; it may be safer to explicitly reset/hide the button in this branch.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic for walking `v4l2_device_list_t`, checking `valid`, and applying `DataManager::isDeviceValid` is duplicated in several places (`videowidget::delayInit`, `videowidget::getFirstValidDevice`, `CMainWindow::setSelBtnShow`, `Camera::refreshCamDevList`); consider extracting a shared helper to produce a filtered list of valid devices so the blacklist behavior stays consistent and is easier to maintain.
- In `CMainWindow::setSelBtnShow`, when `validNum <= 1` you return early without updating `m_bSwitchCameraShowEnable` or hiding the switch button, which could leave a stale visible switch button if the number of valid devices drops at runtime; it may be safer to explicitly reset/hide the button in this branch.
## Individual Comments
### Comment 1
<location path="src/src/camera.cpp" line_range="159-166" />
<code_context>
m_cameraDevList.clear();
v4l2_device_list_t *devlist = get_device_list();
- for (int i = 0; i < devlist->num_devices; i++)
+ if (!devlist) {
+ qWarning() << __func__ << "get device list FAILED!";
+ return;
+ }
+
+ for (int i = 0; i < devlist->num_devices; i++) {
+ // 有黑名单的存在，跳过无效的设备
+ if (!devlist->list_devices[i].valid) {
+ continue;
</code_context>
<issue_to_address>
**issue (bug_risk):** Add a null check for `devlist->list_devices` before iterating, for consistency with other call sites.
Other call sites (e.g. `getFirstValidDevice`, `setSelBtnShow`) also guard `devlist->list_devices` before use. To avoid a possible null dereference from a partially initialized `v4l2_device_list_t`, please also check `devlist->list_devices` here:
```cpp
if (!devlist || !devlist->list_devices) {
qWarning() << __func__ << "get device list FAILED!";
return;
}
```
</issue_to_address>
### Comment 2
<location path="src/main.cpp" line_range="202-206" />
<code_context>
set_enable_8k_preview(enable ? 1 : 0);
}
+ if (dconfig && dconfig->isValid() && dconfig->keyList().contains("deviceBlacklist")) {
+ QStringList deviceBlacklist = dconfig->value("deviceBlacklist").toStringList();
+ qInfo() << "device blacklist:" << deviceBlacklist;
+ DataManager::instance()->setDeviceBlacklist(deviceBlacklist);
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Validate the format of `deviceBlacklist` entries or log malformed entries to make misconfiguration easier to debug.
This relies on every `deviceBlacklist` entry matching the `vid,pid,name` format expected by `isDeviceValid`. Malformed entries (wrong component count, different separators) will just be ignored, making config issues hard to diagnose. Consider validating entries or logging a warning for malformed ones when loading the config.
```suggestion
if (dconfig && dconfig->isValid() && dconfig->keyList().contains("deviceBlacklist")) {
QStringList deviceBlacklist = dconfig->value("deviceBlacklist").toStringList();
QStringList validDeviceBlacklist;
for (const QString &entry : deviceBlacklist) {
const QStringList parts = entry.split(',');
// Expecting "vid,pid,name" format as required by isDeviceValid
if (parts.size() != 3 || parts[0].trimmed().isEmpty() || parts[1].trimmed().isEmpty() || parts[2].trimmed().isEmpty()) {
qWarning() << "Ignoring malformed deviceBlacklist entry:" << entry
<< "- expected format: vid,pid,name";
continue;
}
validDeviceBlacklist << entry;
}
qInfo() << "device blacklist (valid entries):" << validDeviceBlacklist;
DataManager::instance()->setDeviceBlacklist(validDeviceBlacklist);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
lichaofan2008
force-pushed
the
release/eagle
branch
13 times, most recently
from
210a1bc to
55d5c30
Compare
Use the DConfig mechanism to enable the configuration for device blacklist. 使用DConfig机制，开发设备黑名单功能，处在黑名单中的设备不会被应用显示。如果现有设备全部在黑名单中，则会有一个默认设备用于预览。 代码来自xiwo分支。 Bug: https://pms.uniontech.com/bug-view-349401.html
lichaofan2008
force-pushed
the
release/eagle
branch
from
55d5c30 to
21cadc0
Compare
deepin pr auto review代码审查报告总体评价这次代码修改主要是为相机应用添加了设备黑名单功能，整体设计合理，实现较为完整。代码结构清晰，功能模块划分明确，但还有一些可以改进的地方。 具体分析1. 设备黑名单功能实现优点：
改进建议：
// 建议添加更详细的错误信息
if (parts.size() != 3) {
qWarning() << "Drop blacklist item(format error):" << item
<< "Expected format: vid,pid,name";
continue;
}2. 设备管理相关改进优点：
改进建议：
// 建议添加设备列表有效性检查
if (!devList || !devList->list_devices || devList->num_devices <= 0) {
qWarning() << __func__ << "Invalid device list!";
return;
}3. 设备切换逻辑优化优点：
改进建议：
// 建议添加设备切换超时处理
bool videowidget::switchCameraWithTimeout(const char *device, const char *name, int timeoutMs) {
QFuture<bool> future = QtConcurrent::run([this, device, name]() {
return switchCamera(device, name);
});
if (future.waitFor(timeoutMs)) {
return future.result();
}
return false;
}4. 线程安全性优点：
改进建议：
// 建议缩小锁的范围
void videowidget::updateValidDevices() {
// 先获取设备数据
v4l2_device_list_t *devList = get_device_list();
if (!devList) return;
// 处理数据
QVector<ValidDevice> newDevices;
// ...处理逻辑...
// 最后更新数据
{
QWriteLocker locker(&m_mutexValidDevices);
m_validDevices = newDevices;
}
}5. 性能优化优点：
改进建议：
// 建议添加设备状态缓存
QMap<QString, DeviceStatus> m_deviceStatusCache;
void videowidget::updateDeviceStatus(const QString &device) {
// 检查缓存是否过期
if (m_deviceStatusCache.contains(device) &&
!m_deviceStatusCache[device].isExpired()) {
return;
}
// 更新设备状态
DeviceStatus status = checkDeviceStatus(device);
m_deviceStatusCache[device] = status;
}6. 错误处理优点：
改进建议：
// 建议使用自定义异常类
class DeviceException : public std::exception {
public:
DeviceException(const QString &message, DeviceError error)
: m_message(message), m_error(error) {}
const char *what() const noexcept override {
return m_message.toStdString().c_str();
}
DeviceError error() const { return m_error; }
private:
QString m_message;
DeviceError m_error;
};总结这次代码修改整体质量较高，实现了设备黑名单功能，并优化了设备管理逻辑。主要改进方向包括：
这些改进将有助于提高代码的健壮性、性能和可维护性。 |
max-lvs
approved these changes
Mar 19, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lichaofan2008, max-lvs 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
3 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.
Use the DConfig mechanism to enable the configuration for device blacklist.
使用DConfig机制，开发设备黑名单功能，处在黑名单中的设备不会被应用显示。如果现有设备全部在黑名单中，则会有一个默认设备用于预览。
代码来自xiwo分支。
Bug: https://pms.uniontech.com/bug-view-349401.html
Summary by Sourcery
Introduce configurable camera device blacklisting and ensure only valid devices are used for preview and UI controls.
New Features:
Enhancements: