fix: Add network OSD processing #422
Merged
Conversation
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
Reviewer's GuideThis PR adds on-screen display (OSD) handling for airplane mode and WiFi state changes by introducing timers and new event slots in NetworkStateHandler, extends SettingConfig to configure the OSD reset timeout, and tweaks a QML font weight property. Sequence diagram for OSD handling on Airplane Mode property changesequenceDiagram
participant "AirplaneMode DBus Service"
participant "NetworkStateHandler"
participant "OSD DBus Service"
"AirplaneMode DBus Service"->>"NetworkStateHandler": PropertiesChanged (Enabled/WifiEnabled)
alt Enabled changed
"NetworkStateHandler"->>"NetworkStateHandler": onAirplaneModeEnabledChanged(bool)
"NetworkStateHandler"->>"OSD DBus Service": showOSD("AirplaneModeOn"/"AirplaneModeOff")
"NetworkStateHandler"->>"NetworkStateHandler": Start m_resetWifiOSDEnableTimer
else WifiEnabled changed
"NetworkStateHandler"->>"NetworkStateHandler": onAirplaneModeWifiEnabledChanged(bool)
"NetworkStateHandler"->>"NetworkStateHandler": Start m_delayShowWifiOSD
end
ER diagram for SettingConfig OSD timeout propertyerDiagram
SETTINGCONFIG {
int m_resetWifiOSDEnableTimeout
}
Class diagram for updated NetworkStateHandler and SettingConfigclassDiagram
class NetworkStateHandler {
+bool m_wifiOSDEnable
+bool m_wifiEnabled
+QTimer* m_delayShowWifiOSD
+QTimer* m_resetWifiOSDEnableTimer
+void onAirplaneModeWifiEnabledChanged(bool enabled)
+void onAirplaneModeEnabledChanged(bool enabled)
+void onAirplaneModePropertiesChanged(const QString &, const QVariantMap &properties, const QStringList &)
+void showOSD(const QString &signal)
+void resetWifiOSDEnable()
+void delayShowWifiOSD()
+void updateOSDTimer(int interval)
}
class SettingConfig {
+int m_resetWifiOSDEnableTimeout
+int resetWifiOSDEnableTimeout() const
+void resetWifiOSDEnableTimeoutChanged(int)
}
NetworkStateHandler --> SettingConfig : uses
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 there - I've reviewed your changes - here's some feedback:
- The resetWifiOSDEnableTimer is currently connected to delayShowWifiOSD(), but it should call resetWifiOSDEnable() to re-enable the WiFi OSD rather than trigger another show call.
- Consider validating or clamping the configured timeout to ensure updateOSDTimer(interval - 50) can’t result in a negative or zero interval.
- The magic “–50” offset in updateOSDTimer isn’t documented—extract it into a named constant or add a comment explaining why the delay is interval minus 50 ms.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The resetWifiOSDEnableTimer is currently connected to delayShowWifiOSD(), but it should call resetWifiOSDEnable() to re-enable the WiFi OSD rather than trigger another show call.
- Consider validating or clamping the configured timeout to ensure updateOSDTimer(interval - 50) can’t result in a negative or zero interval.
- The magic “–50” offset in updateOSDTimer isn’t documented—extract it into a named constant or add a comment explaining why the delay is interval minus 50 ms.
## Individual Comments
### Comment 1
<location> `network-service-plugin/src/session/networkstatehandler.cpp:729-738` </location>
<code_context>
+ m_wifiOSDEnable = true;
+}
+
+void NetworkStateHandler::delayShowWifiOSD()
+{
+ // 禁用WIFI网络OSD时退出
+ if (!m_wifiOSDEnable) {
+ return;
+ }
+
+ // if enabled is true, wifi rfkill block is true
+ // so wlan is off
+ if (m_wifiEnabled) {
+ showOSD("WLANOff");
+ } else { // if enabled is false, wifi is off
</code_context>
<issue_to_address>
**issue (bug_risk):** Logic for showing WLANOn/WLANOff OSD may be inverted.
Please verify that the OSD display matches the actual WiFi state, as the current logic appears to contradict the variable naming.
</issue_to_address>
### Comment 2
<location> `network-service-plugin/src/session/networkstatehandler.cpp:745-749` </location>
<code_context>
+ }
+}
+
+void NetworkStateHandler::updateOSDTimer(int interval)
+{
+ m_delayShowWifiOSD->setInterval(interval - 50);
+ m_resetWifiOSDEnableTimer->setInterval(interval);
+}
+
</code_context>
<issue_to_address>
**suggestion:** Subtracting 50ms from interval for m_delayShowWifiOSD may cause negative values.
Add a check to ensure the interval does not fall below a safe minimum before subtracting 50ms.
```suggestion
void NetworkStateHandler::updateOSDTimer(int interval)
{
const int minDelayOSDInterval = 50; // Minimum safe interval in ms
int delayOSDInterval = interval - 50;
if (delayOSDInterval < minDelayOSDInterval) {
delayOSDInterval = minDelayOSDInterval;
}
m_delayShowWifiOSD->setInterval(delayOSDInterval);
m_resetWifiOSDEnableTimer->setInterval(interval);
}
```
</issue_to_address>
### Comment 3
<location> `network-service-plugin/src/session/networkstatehandler.cpp:63` </location>
<code_context>
, m_replacesId(0)
+ , m_wifiOSDEnable(true)
+ , m_wifiEnabled(true)
+ , m_delayShowWifiOSD(new QTimer(this))
+ , m_resetWifiOSDEnableTimer(new QTimer(this))
{
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the two timers and multiple flags with a single QTimer and an enum to manage delayed OSD actions.
Here’s one way to collapse those two timers and the flag‐juggling into a single `QTimer` plus a small enum to track what action to take when it fires. It preserves:
- delaying the Wi-Fi OSD
- resetting the “can-show-Wi-Fi‐OSD” flag
- immediate AirplaneMode OSD
```cpp
// in NetworkStateHandler.h
enum class PendingOsd { None, ShowWifiOn, ShowWifiOff, ResetWifiEnable };
QTimer* m_osdTimer = new QTimer(this);
PendingOsd m_pendingOsd = PendingOsd::None;
bool m_canShowWifiOsd = true;
bool m_wifiEnabled = true;
int m_baseInterval; // from settings
```
```cpp
// in ctor – replace both timers
connect(m_osdTimer, &QTimer::timeout, this, &NetworkStateHandler::onOsdTimer);
m_baseInterval = SettingConfig::instance()->resetWifiOSDEnableTimeout();
m_osdTimer->setInterval(m_baseInterval);
```
```cpp
// shared helper to schedule any delayed OSD task
void NetworkStateHandler::scheduleOsd(PendingOsd what, int ms)
{
m_pendingOsd = what;
m_osdTimer->start(ms);
}
void NetworkStateHandler::onAirplaneModeEnabledChanged(bool enabled)
{
// block Wifi-OSD, show Airplane OSD immediately
m_canShowWifiOsd = false;
showOSD(enabled ? "AirplaneModeOn" : "AirplaneModeOff");
// schedule resetting the Wifi-OSD flag
scheduleOsd(PendingOsd::ResetWifiEnable, m_baseInterval);
}
void NetworkStateHandler::onAirplaneModeWifiEnabledChanged(bool enabled)
{
if (!m_canShowWifiOsd)
return;
m_wifiEnabled = enabled;
// wait “m_baseInterval – 50” before showing Wifi OSD
scheduleOsd(enabled ? PendingOsd::ShowWifiOff
: PendingOsd::ShowWifiOn,
m_baseInterval - 50);
}
void NetworkStateHandler::onOsdTimer()
{
switch (m_pendingOsd) {
case PendingOsd::ShowWifiOn:
showOSD("WLANOn");
break;
case PendingOsd::ShowWifiOff:
showOSD("WLANOff");
break;
case PendingOsd::ResetWifiEnable:
m_canShowWifiOsd = true;
break;
case PendingOsd::None: break;
}
m_pendingOsd = PendingOsd::None;
m_osdTimer->stop();
}
```
This removes the second timer and the extra boolean flag, centralizes all delayed work in one place, and keeps exactly the same 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
+729
to
+738
| void NetworkStateHandler::delayShowWifiOSD() | ||
| { | ||
| // 禁用WIFI网络OSD时退出 | ||
| if (!m_wifiOSDEnable) { | ||
| return; | ||
| } | ||
|
|
||
| // if enabled is true, wifi rfkill block is true | ||
| // so wlan is off | ||
| if (m_wifiEnabled) { |
There was a problem hiding this comment.
issue (bug_risk): Logic for showing WLANOn/WLANOff OSD may be inverted.
Please verify that the OSD display matches the actual WiFi state, as the current logic appears to contradict the variable naming.
Comment on lines
745
to
752
| void NetworkStateHandler::updateOSDTimer(int interval) | ||
| { | ||
| m_delayShowWifiOSD->setInterval(interval - 50); | ||
| m_resetWifiOSDEnableTimer->setInterval(interval); | ||
| } |
There was a problem hiding this comment.
suggestion: Subtracting 50ms from interval for m_delayShowWifiOSD may cause negative values.
Add a check to ensure the interval does not fall below a safe minimum before subtracting 50ms.
Suggested change
| void NetworkStateHandler::updateOSDTimer(int interval) | |
| { | |
| m_delayShowWifiOSD->setInterval(interval - 50); | |
| m_resetWifiOSDEnableTimer->setInterval(interval); | |
| } | |
| void NetworkStateHandler::updateOSDTimer(int interval) | |
| { | |
| const int minDelayOSDInterval = 50; // Minimum safe interval in ms | |
| int delayOSDInterval = interval - 50; | |
| if (delayOSDInterval < minDelayOSDInterval) { | |
| delayOSDInterval = minDelayOSDInterval; | |
| } | |
| m_delayShowWifiOSD->setInterval(delayOSDInterval); | |
| m_resetWifiOSDEnableTimer->setInterval(interval); | |
| } |
robertkill
previously approved these changes
Oct 28, 2025
Add network OSD processing pms: BUG-338381
deepin pr auto review我来对这个diff进行详细的代码审查：
font: DccUtils.copyFont(D.DTK.fontManager.t5, {
- "bold": true
+ "weight": 700
})改进建议：
改进建议： a) 构造函数中的定时器初始化： m_delayShowWifiOSD(new QTimer(this))
m_resetWifiOSDEnableTimer(new QTimer(this))
b) updateOSDTimer函数： if (interval < 60) {
interval = 60;
}
m_delayShowWifiOSD->setInterval(interval - 50);
m_resetWifiOSDEnableTimer->setInterval(interval);
c) delayShowWifiOSD函数： if (m_wifiEnabled) {
showOSD("WLANOff");
} else {
showOSD("WLANOn");
}
改进建议：
这些修改主要实现了飞行模式和WiFi状态变化的OSD显示功能，整体设计合理，但在代码细节和错误处理方面还有改进空间。 |
robertkill
approved these changes
Oct 29, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, robertkill 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 |
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.
Add network OSD processing
pms: BUG-338381
Summary by Sourcery
Add on-screen display (OSD) handling for airplane mode and Wi-Fi state changes with configurable delays and throttling to prevent overlapping notifications.
New Features:
Enhancements:
Chores: