Replace ExportTempate createsHWMon boolean with an enumeration type
The createsHWMon boolean in the ExportTemplate structure applies an
important initialization strategy for some I2C devies. When new I2C
devices are added to the map of potential I2C devices it is common for
entries to be copied, and pasted, without review of all of the
parameters.
This change is an attempt to reduce copy/paste errors. An enumeration
provides better guidance for initializing the new entry.
Tested:
Confirmed, during debug, the correct initialization path is taken
based upon the hasHWMonDir value.
Confirmed I2C entries are still initialized.
Change-Id: I9141e1875ee99bfe1e7abdc616fb5ea2446e40ba
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
diff --git a/include/devices.hpp b/include/devices.hpp
index 73c363c..01a6e7a 100644
--- a/include/devices.hpp
+++ b/include/devices.hpp
@@ -29,235 +29,323 @@
}
};
+// I2C device drivers may create a /hwmon subdirectory. For example the tmp75
+// driver creates a /sys/bus/i2c/devices/<busnum>-<i2caddr>/hwmon
+// directory. The sensor code relies on the presence of the /hwmon
+// subdirectory to collect sensor readings. Initialization of this subdir is
+// not reliable. I2C devices flagged with hasHWMonDir are tested for correct
+// initialization, and when a failure is detected the device is deleted, and
+// then recreated. The default is to retry 5 times before moving to the next
+// device.
+
+// Devices such as I2C EEPROMs do not generate this file structure. These
+// kinds of devices are flagged using the noHWMonDir enumeration. The
+// expectation is they are created correctly on the first attempt.
+
+// This enumeration class exists to reduce copy/paste errors. It is easy to
+// overlook the trailing parameter in the ExportTemplate structure when it is
+// a simple boolean.
+enum class createsHWMon : bool
+{
+ noHWMonDir,
+ hasHWMonDir
+};
+
struct ExportTemplate
{
ExportTemplate(const char* params, const char* bus, const char* constructor,
- const char* destructor, bool createsHWMon) :
+ const char* destructor, createsHWMon hasHWMonDir) :
parameters(params),
busPath(bus), add(constructor), remove(destructor),
- createsHWMon(createsHWMon){};
+ hasHWMonDir(hasHWMonDir){};
const char* parameters;
const char* busPath;
const char* add;
const char* remove;
- bool createsHWMon;
+ createsHWMon hasHWMonDir;
};
const boost::container::flat_map<const char*, ExportTemplate, CmpStr>
exportTemplates{
{{"EEPROM_24C01",
ExportTemplate("24c01 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EEPROM_24C02",
ExportTemplate("24c02 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EEPROM_24C04",
ExportTemplate("24c04 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EEPROM_24C08",
ExportTemplate("24c08 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EEPROM_24C16",
ExportTemplate("24c16 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EEPROM_24C32",
ExportTemplate("24c32 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EEPROM_24C64",
ExportTemplate("24c64 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EEPROM_24C128",
ExportTemplate("24c128 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EEPROM_24C256",
ExportTemplate("24c256 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"ADM1266",
ExportTemplate("adm1266 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"ADM1272",
ExportTemplate("adm1272 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"ADS7828",
ExportTemplate("ads7828 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"DPS310",
ExportTemplate("dps310 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"SI7020",
ExportTemplate("si7020 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EEPROM",
ExportTemplate("eeprom $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"EMC1412",
ExportTemplate("emc1412 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"EMC1413",
ExportTemplate("emc1413 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"EMC1414",
ExportTemplate("emc1414 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"Gpio", ExportTemplate("$Index", "/sys/class/gpio", "export",
- "unexport", false)},
+ "unexport", createsHWMon::noHWMonDir)},
{"INA230",
ExportTemplate("ina230 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"ISL68137",
ExportTemplate("isl68137 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"ISL68220",
ExportTemplate("isl68220 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"ISL69225",
ExportTemplate("isl69225 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"ISL68223",
ExportTemplate("isl68223 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"ISL69243",
ExportTemplate("isl69243 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"ISL69260",
ExportTemplate("isl69260 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX16601",
ExportTemplate("max16601 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX20710",
ExportTemplate("max20710 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX20730",
ExportTemplate("max20730 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX20734",
ExportTemplate("max20734 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX20796",
ExportTemplate("max20796 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX31725",
ExportTemplate("max31725 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX31730",
ExportTemplate("max31730 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX34440",
ExportTemplate("max34440 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX34451",
ExportTemplate("max34451 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX6654",
ExportTemplate("max6654 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"NCT6779",
ExportTemplate("nct6779 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"PCA9542Mux",
ExportTemplate("pca9542 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9543Mux",
ExportTemplate("pca9543 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9544Mux",
ExportTemplate("pca9544 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9545Mux",
ExportTemplate("pca9545 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9546Mux",
ExportTemplate("pca9546 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9547Mux",
ExportTemplate("pca9547 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9548Mux",
ExportTemplate("pca9548 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9846Mux",
ExportTemplate("pca9846 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9847Mux",
ExportTemplate("pca9847 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9848Mux",
ExportTemplate("pca9848 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"PCA9849Mux",
ExportTemplate("pca9849 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
- {"SBTSI",
- ExportTemplate("sbtsi $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
+ {"SBTSI", ExportTemplate("sbtsi $Address",
+ "/sys/bus/i2c/devices/i2c-$Bus", "new_device",
+ "delete_device", createsHWMon::hasHWMonDir)},
{"SIC450",
ExportTemplate("sic450 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
- {"pmbus",
- ExportTemplate("pmbus $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
+ {"pmbus", ExportTemplate("pmbus $Address",
+ "/sys/bus/i2c/devices/i2c-$Bus", "new_device",
+ "delete_device", createsHWMon::hasHWMonDir)},
{"PXE1610",
ExportTemplate("pxe1610 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"XDPE12284",
ExportTemplate("xdpe12284 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"LM95234",
ExportTemplate("lm95234 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"TMP112",
ExportTemplate("tmp112 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"TMP175",
ExportTemplate("tmp175 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"TMP421",
ExportTemplate("tmp421 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"TMP441",
ExportTemplate("tmp441 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
- {"LM75A",
- ExportTemplate("lm75a $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
+ {"LM75A", ExportTemplate("lm75a $Address",
+ "/sys/bus/i2c/devices/i2c-$Bus", "new_device",
+ "delete_device", createsHWMon::hasHWMonDir)},
{"LM25066",
ExportTemplate("lm25066 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
- {"TMP75",
- ExportTemplate("tmp75 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
+ {"TMP75", ExportTemplate("tmp75 $Address",
+ "/sys/bus/i2c/devices/i2c-$Bus", "new_device",
+ "delete_device", createsHWMon::hasHWMonDir)},
{"W83773G",
ExportTemplate("w83773g $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"RAA228000",
ExportTemplate("raa228000 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"DPS800",
ExportTemplate("dps800 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MAX31790",
ExportTemplate("max31790 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
- {"JC42",
- ExportTemplate("jc42 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
+ {"JC42", ExportTemplate("jc42 $Address",
+ "/sys/bus/i2c/devices/i2c-$Bus", "new_device",
+ "delete_device", createsHWMon::hasHWMonDir)},
{"ADM1293",
ExportTemplate("adm1293 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"INA219",
ExportTemplate("ina219 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)},
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)},
{"RAA229126",
ExportTemplate("raa229126 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MP2971",
ExportTemplate("mp2971 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"MP2973",
ExportTemplate("mp2973 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", true)},
+ "new_device", "delete_device",
+ createsHWMon::hasHWMonDir)},
{"HDC1080",
ExportTemplate("hdc1080 $Address", "/sys/bus/i2c/devices/i2c-$Bus",
- "new_device", "delete_device", false)}}};
+ "new_device", "delete_device",
+ createsHWMon::noHWMonDir)}}};
} // namespace devices
diff --git a/src/overlay.cpp b/src/overlay.cpp
index 9dfb6b1..6352cba 100644
--- a/src/overlay.cpp
+++ b/src/overlay.cpp
@@ -64,7 +64,8 @@
static std::string deviceDirName(uint64_t bus, uint64_t address)
{
std::ostringstream name;
- name << bus << "-" << std::hex << std::setw(4) << std::setfill('0') << address;
+ name << bus << "-" << std::hex << std::setw(4) << std::setfill('0')
+ << address;
return name.str();
}
@@ -160,7 +161,7 @@
static bool deviceIsCreated(const std::string& busPath,
const std::shared_ptr<uint64_t>& bus,
const std::shared_ptr<uint64_t>& address,
- const bool createsHWMon)
+ const devices::createsHWMon hasHWMonDir)
{
if (!bus || !address)
{
@@ -169,7 +170,7 @@
std::filesystem::path dirPath = busPath;
dirPath /= deviceDirName(*bus, *address);
- if (createsHWMon)
+ if (hasHWMonDir == devices::createsHWMon::hasHWMonDir)
{
dirPath /= "hwmon";
}
@@ -184,7 +185,8 @@
const std::shared_ptr<uint64_t>& bus,
const std::shared_ptr<uint64_t>& address,
const std::string& constructor,
- const std::string& destructor, const bool createsHWMon,
+ const std::string& destructor,
+ const devices::createsHWMon hasHWMonDir,
const size_t retries = 5)
{
if (retries == 0U)
@@ -193,7 +195,7 @@
}
// If it's already instantiated, there's nothing we need to do.
- if (deviceIsCreated(busPath, bus, address, createsHWMon))
+ if (deviceIsCreated(busPath, bus, address, hasHWMonDir))
{
return 0;
}
@@ -202,24 +204,23 @@
createDevice(busPath, parameters, constructor);
// If it didn't work, delete it and try again in 500ms
- if (!deviceIsCreated(busPath, bus, address, createsHWMon))
+ if (!deviceIsCreated(busPath, bus, address, hasHWMonDir))
{
deleteDevice(busPath, address, destructor);
std::shared_ptr<boost::asio::steady_timer> createTimer =
std::make_shared<boost::asio::steady_timer>(io);
createTimer->expires_after(std::chrono::milliseconds(500));
- createTimer->async_wait([createTimer, busPath, parameters, bus,
- address, constructor, destructor, createsHWMon,
+ createTimer->async_wait([createTimer, busPath, parameters, bus, address,
+ constructor, destructor, hasHWMonDir,
retries](const boost::system::error_code& ec) {
if (ec)
{
std::cerr << "Timer error: " << ec << "\n";
return -2;
}
- return buildDevice(busPath, parameters, bus, address,
- constructor, destructor, createsHWMon,
- retries - 1);
+ return buildDevice(busPath, parameters, bus, address, constructor,
+ destructor, hasHWMonDir, retries - 1);
});
}
@@ -235,7 +236,7 @@
std::string busPath = exportTemplate.busPath;
std::string constructor = exportTemplate.add;
std::string destructor = exportTemplate.remove;
- bool createsHWMon = exportTemplate.createsHWMon;
+ devices::createsHWMon hasHWMonDir = exportTemplate.hasHWMonDir;
std::string name = "unknown";
std::shared_ptr<uint64_t> bus = nullptr;
std::shared_ptr<uint64_t> address = nullptr;
@@ -280,7 +281,7 @@
}
int err = buildDevice(busPath, parameters, bus, address, constructor,
- destructor, createsHWMon);
+ destructor, hasHWMonDir);
if ((err == 0) && boost::ends_with(type, "Mux") && bus && address &&
(channels != nullptr))