fix: resolve hotspot device not updating when network interface changes#499
Open
caixr23 wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: resolve hotspot device not updating when network interface changes#499caixr23 wants to merge 1 commit intolinuxdeepin:masterfrom
caixr23 wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23 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 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces the previous QTimer-based debounce/sequencing mechanism in NetHotspotController with a simple integer counter to reliably gate hotspot configuration updates when network interfaces and secrets change, addressing missed hotspot device updates under rapid network changes. Sequence diagram for hotspot configuration updates with counter-based gatingsequenceDiagram
participant NetHotspotController
participant NetworkController
participant Connection
participant WirelessSecuritySetting
NetHotspotController->>NetworkController: hotspotController()
activate NetworkController
NetworkController-->>NetHotspotController: HotspotController
deactivate NetworkController
Note over NetHotspotController: Initial hotspot data/config setup
NetHotspotController->>NetHotspotController: updateData()
NetHotspotController->>NetHotspotController: updateEnabled()
NetHotspotController->>NetHotspotController: updateEnabledable()
NetHotspotController->>NetHotspotController: updateConfig()
alt m_userActive is false or m_updateCount > 0
NetHotspotController->>NetHotspotController: m_updateCount-- (if m_updateCount > 0)
Note over NetHotspotController: Config update deferred
else
NetHotspotController->>Connection: secrets(name)
Connection-->>NetHotspotController: secrets map
NetHotspotController->>WirelessSecuritySetting: secretsFromMap(secrets)
end
Note over Connection: conn->secrets triggers itemChanged twice
loop For each itemChanged signal
Connection-->>NetHotspotController: itemChanged
NetHotspotController->>NetHotspotController: updateData()
NetHotspotController->>NetHotspotController: updateEnabled()
NetHotspotController->>NetHotspotController: updateEnabledable()
NetHotspotController->>NetHotspotController: updateConfig()
alt First secrets handling branch
NetHotspotController->>NetHotspotController: m_updateCount = 2
else Subsequent calls while m_updateCount > 0
NetHotspotController->>NetHotspotController: m_updateCount--
Note over NetHotspotController: Skip duplicate config updates
end
end
Class diagram for updated NetHotspotController debounce mechanismclassDiagram
class NetHotspotController {
- bool m_hotspotEnabled
- bool m_enabledable
- bool m_deviceEnabled
- bool m_userActive
- int m_updateCount
- QStringList m_shareDevice
- QStringList m_optionalDevice
- QStringList m_optionalDevicePath
+ NetHotspotController(QObject *parent)
+ void updateData()
+ void updateConfig()
}
class NetworkController {
+ static NetworkController* instance()
+ HotspotController* hotspotController()
}
class HotspotController {
}
NetHotspotController --> NetworkController : uses
NetworkController --> HotspotController : returns instance
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
m_updateCountlogic inupdateConfig()is a bit opaque; consider replacing the magic value2with a named constant and adding a short comment describing the intended number of skippeditemChangedevents to make the sequencing behavior clearer. - In
updateConfig(),m_updateCount--is executed even when!m_userActive, which can drive the counter negative and change behavior once the user becomes active; it may be safer to only decrement when you actually intend to consume an update (e.g., whenm_userActiveis true).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `m_updateCount` logic in `updateConfig()` is a bit opaque; consider replacing the magic value `2` with a named constant and adding a short comment describing the intended number of skipped `itemChanged` events to make the sequencing behavior clearer.
- In `updateConfig()`, `m_updateCount--` is executed even when `!m_userActive`, which can drive the counter negative and change behavior once the user becomes active; it may be safer to only decrement when you actually intend to consume an update (e.g., when `m_userActive` is true).
## Individual Comments
### Comment 1
<location> `net-view/operation/private/nethotspotcontroller.cpp:138-139` </location>
<code_context>
{
// 用户不为Active状态时，获取密码需要认证
- if (!m_userActive || m_updatedTimer->isActive()) {
+ if (!m_userActive || m_updateCount > 0) {
+ m_updateCount--;
return;
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid decrementing m_updateCount when the user is inactive; split the conditions.
Previously, when `!m_userActive` the function returned without changing any state. Now, every inactive call decrements `m_updateCount`, which can drive it negative or wrap over time. Please separate the checks so inactivity only returns, and decrement happens only when active, e.g.:
```cpp
if (!m_userActive) {
return;
}
if (m_updateCount > 0) {
m_updateCount--;
return;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Changed from QTimer to counter mechanism for handling network configuration updates Removed QTimer and replaced with integer counter to track update cycles Fixed issue where hotspot devices were not properly updated when network interfaces changed The previous timer approach could miss updates if network changes occurred rapidly New counter mechanism ensures proper sequencing of configuration updates Log: Fixed hotspot device update issue when network interfaces change Influence: 1. Test hotspot functionality after network interface changes 2. Verify hotspot configuration updates properly 3. Check that password retrieval works correctly after network changes 4. Test multiple rapid network interface changes 5. Verify user active state handling remains functional fix: 修复网卡变化时热点设备未更新问题 从 QTimer 改为计数器机制处理网络配置更新 移除 QTimer 并使用整数计数器跟踪更新周期 修复当网络接口变化时热点设备未能正确更新的问题 之前的定时器方法在快速网络变化时可能错过更新 新的计数器机制确保配置更新的正确顺序 Log: 修复网卡变化时热点设备更新问题 Influence: 1. 测试网络接口变化后的热点功能 2. 验证热点配置是否正确更新 3. 检查网络变化后密码获取功能是否正常 4. 测试多次快速网络接口变化 5. 验证用户活跃状态处理保持正常 PMS: BUG-303389
deepin pr auto review这段代码的主要改动是将使用 1. 代码逻辑与功能分析
2. 代码质量与可维护性
3. 代码性能
4. 代码安全
改进建议建议 1：保留 QTimer 方案（推荐）
// 在构造函数中
m_updatedTimer = new QTimer(this);
m_updatedTimer->setSingleShot(true);
m_updatedTimer->setInterval(100); // 适当增加间隔，确保两次信号都能被捕获
// 连接处
connect(m_hotspotController, &HotspotController::itemChanged, this, [this] {
// 每次信号到来都重启计时器，只有停止触发 100ms 后才会执行 updateConfig
m_updatedTimer->start();
});
// 连接计时器超时信号到更新函数
connect(m_updatedTimer, &QTimer::timeout, this, &NetHotspotController::updateConfig);建议 2：如果必须使用计数器方案（优化版） 如果确实因为特殊原因必须使用计数器，建议增加保护机制，防止信号丢失或死锁，并添加注释说明原因。 // 头文件
private:
int m_updateCount = 0;
bool m_isIgnoringSignals = false; // 标记是否处于忽略信号的特定流程中
// 源文件 updateConfig 中
// ...
} else {
// conn->secrets会触发两次itemChanged信号，我们需要忽略这两次信号
// 使用标志位确保只忽略 secrets 调用产生的副作用，不影响其他真实变化
m_isIgnoringSignals = true;
m_updateCount = 2;
// ... secrets 调用 ...
}
// 源文件 onItemChanged 中
void NetHotspotController::onItemChanged()
{
if (m_isIgnoringSignals) {
if (m_updateCount > 0) {
m_updateCount--;
} else {
// 预期的忽略次数已结束，恢复正常状态
m_isIgnoringSignals = false;
// 必须显式调用一次更新，因为刚才的信号被消耗了
updateConfig();
}
} else {
// 正常状态下直接更新
updateConfig();
}
}总结： |
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.
Changed from QTimer to counter mechanism for handling network configuration updates
Removed QTimer and replaced with integer counter to track update cycles Fixed issue where hotspot devices were not properly updated when network interfaces changed
The previous timer approach could miss updates if network changes occurred rapidly
New counter mechanism ensures proper sequencing of configuration updates
Log: Fixed hotspot device update issue when network interfaces change
Influence:
fix: 修复网卡变化时热点设备未更新问题
从 QTimer 改为计数器机制处理网络配置更新
移除 QTimer 并使用整数计数器跟踪更新周期
修复当网络接口变化时热点设备未能正确更新的问题
之前的定时器方法在快速网络变化时可能错过更新
新的计数器机制确保配置更新的正确顺序
Log: 修复网卡变化时热点设备更新问题
Influence:
PMS: BUG-303389
Summary by Sourcery
Replace the hotspot configuration update timer with a counter-based mechanism to ensure hotspot devices and configuration are correctly refreshed when network interfaces or connection secrets change.
Bug Fixes:
Enhancements: