Skip to content

Commit 9a286db

Browse files
williamspatrickwilliamspatrick
committed
reduce dbus string constants
There are a large number of dbus constants scattered throughout the code that could/should be obtained from phosphor-dbus-interface values. Perform minor refactoring to greatly reduce the number of string constants. Signed-off-by: Patrick Williams <[email protected]> Change-Id: Ie8700bc90611d21eee7160f4686bc978fe0a0eb4
1 parent 4712084 commit 9a286db

13 files changed

+154
-114
lines changed

‎chassis_state_manager.cpp

+19-16
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
#include <fmt/printf.h>
1111

1212
#include <cereal/archives/json.hpp>
13+
#include <org/freedesktop/UPower/Device/client.hpp>
1314
#include <phosphor-logging/elog-errors.hpp>
1415
#include <phosphor-logging/lg2.hpp>
1516
#include <sdbusplus/bus.hpp>
1617
#include <sdbusplus/exception.hpp>
1718
#include <sdeventplus/event.hpp>
1819
#include <sdeventplus/exception.hpp>
20+
#include <xyz/openbmc_project/ObjectMapper/client.hpp>
1921
#include <xyz/openbmc_project/State/Chassis/error.hpp>
2022
#include <xyz/openbmc_project/State/Decorator/PowerSystemInputs/server.hpp>
2123

@@ -36,6 +38,9 @@ namespace server = sdbusplus::server::xyz::openbmc_project::state;
3638
namespace decoratorServer =
3739
sdbusplus::server::xyz::openbmc_project::state::decorator;
3840

41+
using ObjectMapper = sdbusplus::client::xyz::openbmc_project::ObjectMapper<>;
42+
using UPowerDevice = sdbusplus::client::org::freedesktop::u_power::Device<>;
43+
3944
using namespace phosphor::logging;
4045
using sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
4146
using sdbusplus::xyz::openbmc_project::State::Shutdown::Power::Error::Blackout;
@@ -65,12 +70,6 @@ constexpr auto SYSTEMD_INTERFACE = "org.freedesktop.systemd1.Manager";
6570
constexpr auto SYSTEMD_PROPERTY_IFACE = "org.freedesktop.DBus.Properties";
6671
constexpr auto SYSTEMD_INTERFACE_UNIT = "org.freedesktop.systemd1.Unit";
6772

68-
constexpr auto MAPPER_BUSNAME = "xyz.openbmc_project.ObjectMapper";
69-
constexpr auto MAPPER_PATH = "/xyz/openbmc_project/object_mapper";
70-
constexpr auto MAPPER_INTERFACE = "xyz.openbmc_project.ObjectMapper";
71-
constexpr auto UPOWER_INTERFACE = "org.freedesktop.UPower.Device";
72-
constexpr auto POWERSYSINPUTS_INTERFACE =
73-
"xyz.openbmc_project.State.Decorator.PowerSystemInputs";
7473
constexpr auto PROPERTY_INTERFACE = "org.freedesktop.DBus.Properties";
7574

7675
void Chassis::createSystemdTargetTable()
@@ -92,7 +91,7 @@ void Chassis::determineInitialState()
9291
uPowerPropChangeSignal = std::make_unique<sdbusplus::bus::match_t>(
9392
bus,
9493
sdbusplus::bus::match::rules::propertiesChangedNamespace(
95-
"/org/freedesktop/UPower", UPOWER_INTERFACE),
94+
"/org/freedesktop/UPower", UPowerDevice::interface),
9695
[this](auto& msg) { this->uPowerChangeEvent(msg); });
9796

9897
// Monitor for any properties changed signals on PowerSystemInputs
@@ -101,7 +100,7 @@ void Chassis::determineInitialState()
101100
sdbusplus::bus::match::rules::propertiesChangedNamespace(
102101
fmt::format(
103102
"/xyz/openbmc_project/power/power_supplies/chassis{}/psus", id),
104-
POWERSYSINPUTS_INTERFACE),
103+
decoratorServer::PowerSystemInputs::interface),
105104
[this](auto& msg) { this->powerSysInputsChangeEvent(msg); });
106105

107106
determineStatusOfPower();
@@ -231,10 +230,11 @@ void Chassis::determineStatusOfPower()
231230
bool Chassis::determineStatusOfUPSPower()
232231
{
233232
// Find all implementations of the UPower interface
234-
auto mapper = bus.new_method_call(MAPPER_BUSNAME, MAPPER_PATH,
235-
MAPPER_INTERFACE, "GetSubTree");
233+
auto mapper = bus.new_method_call(ObjectMapper::default_service,
234+
ObjectMapper::instance_path,
235+
ObjectMapper::interface, "GetSubTree");
236236

237-
mapper.append("/", 0, std::vector<std::string>({UPOWER_INTERFACE}));
237+
mapper.append("/", 0, std::vector<std::string>({UPowerDevice::interface}));
238238

239239
std::map<std::string, std::map<std::string, std::vector<std::string>>>
240240
mapperResponse;
@@ -266,7 +266,7 @@ bool Chassis::determineStatusOfUPSPower()
266266
{
267267
auto method = bus.new_method_call(service.c_str(), path.c_str(),
268268
PROPERTY_INTERFACE, "GetAll");
269-
method.append(UPOWER_INTERFACE);
269+
method.append(UPowerDevice::interface);
270270

271271
auto response = bus.call(method);
272272
using Property = std::string;
@@ -337,10 +337,13 @@ bool Chassis::determineStatusOfUPSPower()
337337
bool Chassis::determineStatusOfPSUPower()
338338
{
339339
// Find all implementations of the PowerSystemInputs interface
340-
auto mapper = bus.new_method_call(MAPPER_BUSNAME, MAPPER_PATH,
341-
MAPPER_INTERFACE, "GetSubTree");
340+
auto mapper = bus.new_method_call(ObjectMapper::default_service,
341+
ObjectMapper::instance_path,
342+
ObjectMapper::interface, "GetSubTree");
342343

343-
mapper.append("/", 0, std::vector<std::string>({POWERSYSINPUTS_INTERFACE}));
344+
mapper.append("/", 0,
345+
std::vector<std::string>(
346+
{decoratorServer::PowerSystemInputs::interface}));
344347

345348
std::map<std::string, std::map<std::string, std::vector<std::string>>>
346349
mapperResponse;
@@ -367,7 +370,7 @@ bool Chassis::determineStatusOfPSUPower()
367370
{
368371
auto method = bus.new_method_call(service.c_str(), path.c_str(),
369372
PROPERTY_INTERFACE, "GetAll");
370-
method.append(POWERSYSINPUTS_INTERFACE);
373+
method.append(decoratorServer::PowerSystemInputs::interface);
371374

372375
auto response = bus.call(method);
373376
using Property = std::string;

‎chassis_state_manager.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33
#include "config.h"
44

55
#include "utils.hpp"
6-
#include "xyz/openbmc_project/State/Chassis/server.hpp"
7-
#include "xyz/openbmc_project/State/PowerOnHours/server.hpp"
86

97
#include <cereal/cereal.hpp>
108
#include <sdbusplus/bus.hpp>
119
#include <sdeventplus/clock.hpp>
1210
#include <sdeventplus/event.hpp>
1311
#include <sdeventplus/utility/timer.hpp>
12+
#include <xyz/openbmc_project/State/Chassis/server.hpp>
13+
#include <xyz/openbmc_project/State/PowerOnHours/server.hpp>
1414

1515
#include <chrono>
1616
#include <filesystem>

‎discover_system_state.cpp

+18-9
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#include <phosphor-logging/lg2.hpp>
1616
#include <sdbusplus/exception.hpp>
1717
#include <sdbusplus/server.hpp>
18+
#include <xyz/openbmc_project/State/BMC/client.hpp>
19+
#include <xyz/openbmc_project/State/Host/client.hpp>
1820

1921
#include <filesystem>
2022
#include <iostream>
@@ -44,7 +46,6 @@ int main(int argc, char** argv)
4446
using namespace phosphor::logging;
4547

4648
size_t hostId = 0;
47-
std::string hostPath = "/xyz/openbmc_project/state/host0";
4849
int arg;
4950
int optIndex = 0;
5051

@@ -57,14 +58,17 @@ int main(int argc, char** argv)
5758
{
5859
case 'h':
5960
hostId = std::stoul(optarg);
60-
hostPath = std::string("/xyz/openbmc_project/state/host") +
61-
optarg;
6261
break;
6362
default:
6463
break;
6564
}
6665
}
6766

67+
using Host = sdbusplus::client::xyz::openbmc_project::state::Host<>;
68+
std::string hostPath = std::string(Host::namespace_path::value) + "/" +
69+
std::string(Host::namespace_path::host) +
70+
std::to_string(hostId);
71+
6872
auto bus = sdbusplus::bus::new_default();
6973

7074
using namespace settings;
@@ -77,17 +81,22 @@ int main(int argc, char** argv)
7781

7882
// If the BMC was rebooted due to a user initiated pinhole reset, do not
7983
// implement any power restore policies
80-
auto bmcRebootCause = phosphor::state::manager::utils::getProperty(
81-
bus, "/xyz/openbmc_project/state/bmc0", BMC_BUSNAME, "LastRebootCause");
82-
if (bmcRebootCause ==
83-
"xyz.openbmc_project.State.BMC.RebootCause.PinholeReset")
84+
using BMC = sdbusplus::client::xyz::openbmc_project::state::BMC<>;
85+
auto bmcPath = sdbusplus::message::object_path(BMC::namespace_path::value) /
86+
BMC::namespace_path::bmc;
87+
88+
auto bmcRebootCause =
89+
sdbusplus::message::convert_from_string<BMC::RebootCause>(
90+
phosphor::state::manager::utils::getProperty(
91+
bus, bmcPath.str.c_str(), BMC_BUSNAME, "LastRebootCause"));
92+
93+
if (bmcRebootCause == BMC::RebootCause::PinholeReset)
8494
{
8595
info(
8696
"BMC was reset due to pinhole reset, no power restore policy will be run");
8797
return 0;
8898
}
89-
else if (bmcRebootCause ==
90-
"xyz.openbmc_project.State.BMC.RebootCause.Watchdog")
99+
else if (bmcRebootCause == BMC::RebootCause::Watchdog)
91100
{
92101
info(
93102
"BMC was reset due to cold reset, no power restore policy will be run");

‎host_check.cpp

+24-26
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@
88
#include <phosphor-logging/lg2.hpp>
99
#include <sdbusplus/bus.hpp>
1010
#include <sdbusplus/exception.hpp>
11-
#include <xyz/openbmc_project/Condition/HostFirmware/server.hpp>
12-
#include <xyz/openbmc_project/State/Chassis/server.hpp>
11+
#include <xyz/openbmc_project/Condition/HostFirmware/client.hpp>
12+
#include <xyz/openbmc_project/ObjectMapper/client.hpp>
13+
#include <xyz/openbmc_project/State/Chassis/client.hpp>
1314

1415
#include <cstdio>
1516
#include <cstdlib>
@@ -28,32 +29,29 @@ namespace manager
2829
PHOSPHOR_LOG2_USING;
2930

3031
using namespace std::literals;
31-
using namespace sdbusplus::server::xyz::openbmc_project::condition;
32+
33+
using ObjectMapper = sdbusplus::client::xyz::openbmc_project::ObjectMapper<>;
34+
using Chassis = sdbusplus::client::xyz::openbmc_project::state::Chassis<>;
35+
using HostFirmware =
36+
sdbusplus::client::xyz::openbmc_project::condition::HostFirmware<>;
3237

3338
// Required strings for sending the msg to check on host
34-
constexpr auto MAPPER_BUSNAME = "xyz.openbmc_project.ObjectMapper";
35-
constexpr auto MAPPER_PATH = "/xyz/openbmc_project/object_mapper";
36-
constexpr auto MAPPER_INTERFACE = "xyz.openbmc_project.ObjectMapper";
37-
constexpr auto CONDITION_HOST_INTERFACE =
38-
"xyz.openbmc_project.Condition.HostFirmware";
3939
constexpr auto CONDITION_HOST_PROPERTY = "CurrentFirmwareCondition";
4040
constexpr auto PROPERTY_INTERFACE = "org.freedesktop.DBus.Properties";
4141

4242
constexpr auto CHASSIS_STATE_SVC = "xyz.openbmc_project.State.Chassis";
43-
constexpr auto CHASSIS_STATE_PATH = "/xyz/openbmc_project/state/chassis";
44-
constexpr auto CHASSIS_STATE_INTF = "xyz.openbmc_project.State.Chassis";
4543
constexpr auto CHASSIS_STATE_POWER_PROP = "CurrentPowerState";
4644

4745
// Find all implementations of Condition interface and check if host is
4846
// running over it
4947
bool checkFirmwareConditionRunning(sdbusplus::bus_t& bus)
5048
{
51-
using FirmwareCondition = HostFirmware::FirmwareCondition;
5249
// Find all implementations of host firmware condition interface
53-
auto mapper = bus.new_method_call(MAPPER_BUSNAME, MAPPER_PATH,
54-
MAPPER_INTERFACE, "GetSubTree");
50+
auto mapper = bus.new_method_call(ObjectMapper::default_service,
51+
ObjectMapper::instance_path,
52+
ObjectMapper::interface, "GetSubTree");
5553

56-
mapper.append("/", 0, std::vector<std::string>({CONDITION_HOST_INTERFACE}));
54+
mapper.append("/", 0, std::vector<std::string>({HostFirmware::interface}));
5755

5856
std::map<std::string, std::map<std::string, std::vector<std::string>>>
5957
mapperResponse;
@@ -97,21 +95,20 @@ bool checkFirmwareConditionRunning(sdbusplus::bus_t& bus)
9795
{
9896
auto method = bus.new_method_call(service.c_str(), path.c_str(),
9997
PROPERTY_INTERFACE, "Get");
100-
method.append(CONDITION_HOST_INTERFACE,
101-
CONDITION_HOST_PROPERTY);
98+
method.append(HostFirmware::interface, CONDITION_HOST_PROPERTY);
10299

103100
auto response = bus.call(method);
104-
std::variant<FirmwareCondition> currentFwCondV;
101+
std::variant<HostFirmware::FirmwareCondition> currentFwCondV;
105102
response.read(currentFwCondV);
106103
auto currentFwCond =
107-
std::get<FirmwareCondition>(currentFwCondV);
104+
std::get<HostFirmware::FirmwareCondition>(currentFwCondV);
108105

109106
info(
110107
"Read host fw condition {COND_VALUE} from {COND_SERVICE}, {COND_PATH}",
111108
"COND_VALUE", currentFwCond, "COND_SERVICE", service,
112109
"COND_PATH", path);
113110

114-
if (currentFwCond == FirmwareCondition::Running)
111+
if (currentFwCond == HostFirmware::FirmwareCondition::Running)
115112
{
116113
return true;
117114
}
@@ -132,23 +129,24 @@ bool checkFirmwareConditionRunning(sdbusplus::bus_t& bus)
132129
bool isChassiPowerOn(sdbusplus::bus_t& bus, size_t id)
133130
{
134131
auto svcname = std::string{CHASSIS_STATE_SVC} + std::to_string(id);
135-
auto objpath = std::string{CHASSIS_STATE_PATH} + std::to_string(id);
132+
auto objpath = std::string{Chassis::namespace_path::value} + "/" +
133+
std::string{Chassis::namespace_path::chassis} +
134+
std::to_string(id);
136135

137136
try
138137
{
139-
using PowerState =
140-
sdbusplus::server::xyz::openbmc_project::state::Chassis::PowerState;
141138
auto method = bus.new_method_call(svcname.c_str(), objpath.c_str(),
142139
PROPERTY_INTERFACE, "Get");
143-
method.append(CHASSIS_STATE_INTF, CHASSIS_STATE_POWER_PROP);
140+
method.append(Chassis::interface, CHASSIS_STATE_POWER_PROP);
144141

145142
auto response = bus.call(method);
146-
std::variant<PowerState> currentPowerStateV;
143+
std::variant<Chassis::PowerState> currentPowerStateV;
147144
response.read(currentPowerStateV);
148145

149-
auto currentPowerState = std::get<PowerState>(currentPowerStateV);
146+
auto currentPowerState =
147+
std::get<Chassis::PowerState>(currentPowerStateV);
150148

151-
if (currentPowerState == PowerState::On)
149+
if (currentPowerState == Chassis::PowerState::On)
152150
{
153151
return true;
154152
}

‎host_reset_recovery.cpp

+17-19
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
#include <phosphor-logging/lg2.hpp>
77
#include <sdbusplus/bus.hpp>
88
#include <sdbusplus/exception.hpp>
9-
#include <xyz/openbmc_project/Logging/Create/server.hpp>
10-
#include <xyz/openbmc_project/Logging/Entry/server.hpp>
11-
#include <xyz/openbmc_project/State/Boot/Progress/server.hpp>
9+
#include <xyz/openbmc_project/Logging/Create/client.hpp>
10+
#include <xyz/openbmc_project/Logging/Entry/client.hpp>
11+
#include <xyz/openbmc_project/State/Boot/Progress/client.hpp>
1212

1313
#include <cstdlib>
1414
#include <fstream>
@@ -23,16 +23,17 @@ namespace manager
2323

2424
PHOSPHOR_LOG2_USING;
2525

26+
using BootProgress =
27+
sdbusplus::client::xyz::openbmc_project::state::boot::Progress<>;
28+
using LoggingCreate =
29+
sdbusplus::client::xyz::openbmc_project::logging::Create<>;
30+
using LoggingEntry = sdbusplus::client::xyz::openbmc_project::logging::Entry<>;
31+
2632
constexpr auto HOST_STATE_SVC = "xyz.openbmc_project.State.Host";
2733
constexpr auto HOST_STATE_PATH = "/xyz/openbmc_project/state/host0";
2834
constexpr auto PROPERTY_INTERFACE = "org.freedesktop.DBus.Properties";
29-
constexpr auto BOOT_STATE_INTF = "xyz.openbmc_project.State.Boot.Progress";
3035
constexpr auto BOOT_PROGRESS_PROP = "BootProgress";
3136

32-
constexpr auto LOGGING_SVC = "xyz.openbmc_project.Logging";
33-
constexpr auto LOGGING_PATH = "/xyz/openbmc_project/logging";
34-
constexpr auto LOGGING_CREATE_INTF = "xyz.openbmc_project.Logging.Create";
35-
3637
constexpr auto SYSTEMD_SERVICE = "org.freedesktop.systemd1";
3738
constexpr auto SYSTEMD_OBJ_PATH = "/org/freedesktop/systemd1";
3839
constexpr auto SYSTEMD_INTERFACE = "org.freedesktop.systemd1.Manager";
@@ -42,12 +43,11 @@ bool wasHostBooting(sdbusplus::bus_t& bus)
4243
{
4344
try
4445
{
45-
using ProgressStages = sdbusplus::xyz::openbmc_project::State::Boot::
46-
server::Progress::ProgressStages;
46+
using ProgressStages = BootProgress::ProgressStages;
4747

4848
auto method = bus.new_method_call(HOST_STATE_SVC, HOST_STATE_PATH,
4949
PROPERTY_INTERFACE, "Get");
50-
method.append(BOOT_STATE_INTF, BOOT_PROGRESS_PROP);
50+
method.append(BootProgress::interface, BOOT_PROGRESS_PROP);
5151

5252
auto response = bus.call(method);
5353

@@ -86,21 +86,19 @@ void createErrorLog(sdbusplus::bus_t& bus)
8686

8787
static constexpr auto errorMessage =
8888
"xyz.openbmc_project.State.Error.HostNotRunning";
89-
auto method = bus.new_method_call(LOGGING_SVC, LOGGING_PATH,
90-
LOGGING_CREATE_INTF, "Create");
91-
method.append(errorMessage,
92-
sdbusplus::server::xyz::openbmc_project::logging::Entry::
93-
Level::Error,
94-
additionalData);
89+
auto method = bus.new_method_call(LoggingCreate::default_service,
90+
LoggingCreate::instance_path,
91+
LoggingCreate::interface, "Create");
92+
method.append(errorMessage, LoggingEntry::Level::Error, additionalData);
9593
auto resp = bus.call(method);
9694
}
9795
catch (const sdbusplus::exception_t& e)
9896
{
9997
error(
10098
"sdbusplus D-Bus call exception, error {ERROR}, objpath {OBJPATH}, "
10199
"interface {INTERFACE}",
102-
"ERROR", e, "OBJPATH", LOGGING_PATH, "INTERFACE",
103-
LOGGING_CREATE_INTF);
100+
"ERROR", e, "OBJPATH", LoggingCreate::instance_path, "INTERFACE",
101+
LoggingCreate::interface);
104102

105103
throw std::runtime_error(
106104
"Error in invoking D-Bus logging create interface");

‎host_state_manager.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44

55
#include "settings.hpp"
66
#include "utils.hpp"
7-
#include "xyz/openbmc_project/State/Host/server.hpp"
87

98
#include <cereal/access.hpp>
109
#include <cereal/cereal.hpp>
1110
#include <phosphor-logging/lg2.hpp>
1211
#include <sdbusplus/bus.hpp>
1312
#include <xyz/openbmc_project/Control/Boot/RebootAttempts/server.hpp>
1413
#include <xyz/openbmc_project/State/Boot/Progress/server.hpp>
14+
#include <xyz/openbmc_project/State/Host/server.hpp>
1515
#include <xyz/openbmc_project/State/OperatingSystem/Status/server.hpp>
1616

1717
#include <filesystem>

0 commit comments

Comments
 (0)