Skip to content

Commit 901314a

Browse files
picnoirpicnoir
committed
modules/power_profiles_daemon: safely call dbus asynchronously
2 changes to address the review feedback: 1. Aleksei pointed out in this comment (Alexays#2971 (comment)) that there's no way to tell if a proxy is alive other than trying to call a method on it. We perform a little dance to check whether or not power-profiles-daemon is available on the system by calling properties.GetAll. If something responds, we assume power-profiles-daemon is installed, it's then safe to draw the widget and attach the callback to the active profile. 2. We replaced all the synchronous DBus operations by their async counterparts.
1 parent 61fed6a commit 901314a

File tree

2 files changed

+132
-72
lines changed

2 files changed

+132
-72
lines changed

‎include/modules/power_profiles_daemon.hpp

+11-5
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,24 @@ struct Profile {
1414

1515
class PowerProfilesDaemon : public ALabel {
1616
public:
17-
PowerProfilesDaemon(const std::string&, const Json::Value&);
17+
PowerProfilesDaemon(const std::string &, const Json::Value &);
1818
~PowerProfilesDaemon() override;
1919
auto update() -> void override;
20-
void profileChangedCb(const Gio::DBus::Proxy::MapChangedProperties&,
21-
const std::vector<Glib::ustring>&);
20+
void profileChangedCb(const Gio::DBus::Proxy::MapChangedProperties &,
21+
const std::vector<Glib::ustring> &);
22+
void busConnectedCb(Glib::RefPtr<Gio::AsyncResult> &r);
23+
void getAllPropsCb(Glib::RefPtr<Gio::AsyncResult> &r);
24+
void setPropCb(Glib::RefPtr<Gio::AsyncResult> &r);
2225
void populateInitState();
23-
bool handleToggle(GdkEventButton* const& e) override;
26+
bool handleToggle(GdkEventButton *const &e) override;
2427

2528
private:
29+
// True if we're connected to the dbug interface. False if we're
30+
// not.
31+
bool connected_;
2632
// Look for a profile name in the list of available profiles and
2733
// switch activeProfile_ to it.
28-
void switchToProfile(std::string const&);
34+
void switchToProfile(std::string const &);
2935
// Used to toggle/display the profiles
3036
std::vector<Profile> availableProfiles_;
3137
// Points to the active profile in the profiles list

‎src/modules/power_profiles_daemon.cpp

+121-67
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
namespace waybar::modules {
1717

1818
PowerProfilesDaemon::PowerProfilesDaemon(const std::string& id, const Json::Value& config)
19-
: ALabel(config, "power-profiles-daemon", id, "{profile}", 0, false, true) {
19+
: ALabel(config, "power-profiles-daemon", id, "{profile}", 0, false, true), connected_(false) {
2020
if (config_["format"].isString()) {
2121
format_ = config_["format"].asString();
2222
} else {
@@ -28,6 +28,20 @@ PowerProfilesDaemon::PowerProfilesDaemon(const std::string& id, const Json::Valu
2828
} else {
2929
tooltipFormat_ = "Power profile: {profile}\nDriver: {driver}";
3030
}
31+
// Fasten your seatbelt, we're up for quite a ride. The rest of the
32+
// init is performed asynchronously. There's 2 callbacks involved.
33+
// Here's the overall idea:
34+
// 1. Async connect to the system bus.
35+
// 2. In the system bus connect callback, try to call
36+
// org.freedesktop.DBus.Properties.GetAll to see if
37+
// power-profiles-daemon is able to respond.
38+
// 3. In the GetAll callback, connect the activeProfile monitoring
39+
// callback, consider the init to be successful. Meaning start
40+
// drawing the module.
41+
//
42+
// There's sadly no other way around that, we have to try to call a
43+
// method on the proxy to see whether or not something's responding
44+
// on the other side.
3145

3246
// NOTE: the DBus adresses are under migration. They should be
3347
// changed to org.freedesktop.UPower.PowerProfiles at some point.
@@ -39,29 +53,52 @@ PowerProfilesDaemon::PowerProfilesDaemon(const std::string& id, const Json::Valu
3953
// adresses for compatibility sake.
4054
//
4155
// Revisit this in 2026, systems should be updated by then.
42-
powerProfilesProxy_ = Gio::DBus::Proxy::create_for_bus_sync(
43-
Gio::DBus::BusType::BUS_TYPE_SYSTEM, "net.hadess.PowerProfiles", "/net/hadess/PowerProfiles",
44-
"net.hadess.PowerProfiles");
45-
if (!powerProfilesProxy_) {
46-
spdlog::error("PowerProfilesDaemon: DBus error, cannot connect to net.hasdess.PowerProfile");
47-
} else {
56+
Gio::DBus::Proxy::create_for_bus(Gio::DBus::BusType::BUS_TYPE_SYSTEM, "net.hadess.PowerProfiles",
57+
"/net/hadess/PowerProfiles", "net.hadess.PowerProfiles",
58+
sigc::mem_fun(*this, &PowerProfilesDaemon::busConnectedCb));
59+
}
60+
61+
PowerProfilesDaemon::~PowerProfilesDaemon() {
62+
if (powerProfileChangeSignal_.connected()) {
63+
powerProfileChangeSignal_.disconnect();
64+
}
65+
if (powerProfilesProxy_) {
66+
powerProfilesProxy_.reset();
67+
}
68+
}
69+
70+
void PowerProfilesDaemon::busConnectedCb(Glib::RefPtr<Gio::AsyncResult>& r) {
71+
try {
72+
powerProfilesProxy_ = Gio::DBus::Proxy::create_for_bus_finish(r);
73+
using GetAllProfilesVar = Glib::Variant<std::tuple<Glib::ustring>>;
74+
auto callArgs = GetAllProfilesVar::create(std::make_tuple("net.hadess.PowerProfiles"));
75+
76+
auto container = Glib::VariantBase::cast_dynamic<Glib::VariantContainerBase>(callArgs);
77+
powerProfilesProxy_->call("org.freedesktop.DBus.Properties.GetAll",
78+
sigc::mem_fun(*this, &PowerProfilesDaemon::getAllPropsCb), container);
4879
// Connect active profile callback
80+
} catch (const std::exception& e) {
81+
spdlog::error("Failed to create the power profiles daemon DBus proxy");
82+
}
83+
}
84+
85+
// Callback for the GetAll call.
86+
//
87+
// We're abusing this call to make sure power-profiles-daemon is
88+
// available on the host. We're not really using
89+
void PowerProfilesDaemon::getAllPropsCb(Glib::RefPtr<Gio::AsyncResult>& r) {
90+
try {
91+
auto _result = powerProfilesProxy_->call_finish(r);
92+
// Power-profiles-daemon responded something, we can assume it's
93+
// available, we can safely attach the activeProfile monitoring
94+
// now.
95+
connected_ = true;
4996
powerProfileChangeSignal_ = powerProfilesProxy_->signal_properties_changed().connect(
5097
sigc::mem_fun(*this, &PowerProfilesDaemon::profileChangedCb));
5198
populateInitState();
5299
dp.emit();
53-
}
54-
}
55-
56-
// Look for the profile str in our internal profiles list. Using a
57-
// vector to store the profiles ain't the smartest move
58-
// complexity-wise, but it makes toggling between the mode easy. This
59-
// vector is 3 elements max, we'll be fine :P
60-
void PowerProfilesDaemon::switchToProfile(std::string const& str) {
61-
auto pred = [str](Profile const& p) { return p.name == str; };
62-
this->activeProfile_ = std::find_if(availableProfiles_.begin(), availableProfiles_.end(), pred);
63-
if (activeProfile_ == availableProfiles_.end()) {
64-
throw std::runtime_error("FATAL, can't find the active profile in the available profiles list");
100+
} catch (const std::exception& err) {
101+
spdlog::error("Failed to query power-profiles-daemon via dbus: {}", err.what());
65102
}
66103
}
67104

@@ -95,68 +132,85 @@ void PowerProfilesDaemon::populateInitState() {
95132
update();
96133
}
97134

98-
PowerProfilesDaemon::~PowerProfilesDaemon() {
99-
if (powerProfileChangeSignal_.connected()) {
100-
powerProfileChangeSignal_.disconnect();
101-
}
102-
if (powerProfilesProxy_) {
103-
powerProfilesProxy_.reset();
104-
}
105-
}
106-
107135
void PowerProfilesDaemon::profileChangedCb(
108136
const Gio::DBus::Proxy::MapChangedProperties& changedProperties,
109137
const std::vector<Glib::ustring>& invalidatedProperties) {
110-
if (auto activeProfileVariant = changedProperties.find("ActiveProfile");
111-
activeProfileVariant != changedProperties.end()) {
112-
std::string activeProfile =
113-
Glib::VariantBase::cast_dynamic<Glib::Variant<std::string>>(activeProfileVariant->second)
114-
.get();
115-
switchToProfile(activeProfile);
116-
update();
138+
// We're likely connected if this callback gets triggered.
139+
// But better be safe than sorry.
140+
if (connected_) {
141+
if (auto activeProfileVariant = changedProperties.find("ActiveProfile");
142+
activeProfileVariant != changedProperties.end()) {
143+
std::string activeProfile =
144+
Glib::VariantBase::cast_dynamic<Glib::Variant<std::string>>(activeProfileVariant->second)
145+
.get();
146+
switchToProfile(activeProfile);
147+
update();
148+
}
117149
}
118150
}
119151

120-
auto PowerProfilesDaemon::update() -> void {
121-
auto profile = (*activeProfile_);
122-
// Set label
123-
fmt::dynamic_format_arg_store<fmt::format_context> store;
124-
store.push_back(fmt::arg("profile", profile.name));
125-
store.push_back(fmt::arg("driver", profile.driver));
126-
store.push_back(fmt::arg("icon", getIcon(0, profile.name)));
127-
label_.set_markup(fmt::vformat(format_, store));
128-
if (tooltipEnabled()) {
129-
label_.set_tooltip_text(fmt::vformat(tooltipFormat_, store));
152+
// Look for the profile str in our internal profiles list. Using a
153+
// vector to store the profiles ain't the smartest move
154+
// complexity-wise, but it makes toggling between the mode easy. This
155+
// vector is 3 elements max, we'll be fine :P
156+
void PowerProfilesDaemon::switchToProfile(std::string const& str) {
157+
auto pred = [str](Profile const& p) { return p.name == str; };
158+
this->activeProfile_ = std::find_if(availableProfiles_.begin(), availableProfiles_.end(), pred);
159+
if (activeProfile_ == availableProfiles_.end()) {
160+
throw std::runtime_error("FATAL, can't find the active profile in the available profiles list");
130161
}
162+
}
131163

132-
// Set CSS class
133-
if (!currentStyle_.empty()) {
134-
label_.get_style_context()->remove_class(currentStyle_);
135-
}
136-
label_.get_style_context()->add_class(profile.name);
137-
currentStyle_ = profile.name;
164+
auto PowerProfilesDaemon::update() -> void {
165+
if (connected_) {
166+
auto profile = (*activeProfile_);
167+
// Set label
168+
fmt::dynamic_format_arg_store<fmt::format_context> store;
169+
store.push_back(fmt::arg("profile", profile.name));
170+
store.push_back(fmt::arg("driver", profile.driver));
171+
store.push_back(fmt::arg("icon", getIcon(0, profile.name)));
172+
label_.set_markup(fmt::vformat(format_, store));
173+
if (tooltipEnabled()) {
174+
label_.set_tooltip_text(fmt::vformat(tooltipFormat_, store));
175+
}
176+
177+
// Set CSS class
178+
if (!currentStyle_.empty()) {
179+
label_.get_style_context()->remove_class(currentStyle_);
180+
}
181+
label_.get_style_context()->add_class(profile.name);
182+
currentStyle_ = profile.name;
138183

139-
ALabel::update();
184+
ALabel::update();
185+
}
140186
}
141187

142188
bool PowerProfilesDaemon::handleToggle(GdkEventButton* const& e) {
143-
if (e->type == GdkEventType::GDK_BUTTON_PRESS && powerProfilesProxy_) {
144-
activeProfile_++;
145-
if (activeProfile_ == availableProfiles_.end()) {
146-
activeProfile_ = availableProfiles_.begin();
189+
if (connected_) {
190+
if (e->type == GdkEventType::GDK_BUTTON_PRESS && powerProfilesProxy_) {
191+
activeProfile_++;
192+
if (activeProfile_ == availableProfiles_.end()) {
193+
activeProfile_ = availableProfiles_.begin();
194+
}
195+
196+
using VarStr = Glib::Variant<Glib::ustring>;
197+
using SetPowerProfileVar = Glib::Variant<std::tuple<Glib::ustring, Glib::ustring, VarStr>>;
198+
VarStr activeProfileVariant = VarStr::create(activeProfile_->name);
199+
auto callArgs = SetPowerProfileVar::create(
200+
std::make_tuple("net.hadess.PowerProfiles", "ActiveProfile", activeProfileVariant));
201+
auto container = Glib::VariantBase::cast_dynamic<Glib::VariantContainerBase>(callArgs);
202+
powerProfilesProxy_->call("org.freedesktop.DBus.Properties.Set",
203+
sigc::mem_fun(*this, &PowerProfilesDaemon::setPropCb), container);
147204
}
148-
149-
using VarStr = Glib::Variant<Glib::ustring>;
150-
using SetPowerProfileVar = Glib::Variant<std::tuple<Glib::ustring, Glib::ustring, VarStr>>;
151-
VarStr activeProfileVariant = VarStr::create(activeProfile_->name);
152-
auto callArgs = SetPowerProfileVar::create(
153-
std::make_tuple("net.hadess.PowerProfiles", "ActiveProfile", activeProfileVariant));
154-
auto container = Glib::VariantBase::cast_dynamic<Glib::VariantContainerBase>(callArgs);
155-
powerProfilesProxy_->call_sync("org.freedesktop.DBus.Properties.Set", container);
156-
157-
update();
205+
return true;
206+
} else {
207+
return true;
158208
}
159-
return true;
209+
}
210+
211+
void PowerProfilesDaemon::setPropCb(Glib::RefPtr<Gio::AsyncResult>& r) {
212+
auto _ = powerProfilesProxy_->call_finish(r);
213+
update();
160214
}
161215

162216
} // namespace waybar::modules

0 commit comments

Comments
 (0)