MCP9808: refactor driver based on feedback

- Remove enum class Register : uint8_t
- Explicitly initialize buffer
- Explicitly initialize temp_raw
- Rename uorb publisher
- Remove overide from print_status()
- Take timestamp after read, correct measurement_time to uint64 and use hrt_elapsed_time()
- Remove exit_and_cleanup
- Move functions out of main + cleanup whitespace

MCP9808: remove exit_and_cleanup

MCP9808: Take timestamp after read, correct measurement_time to uint64 and use hrt_elapsed_time()

MCP9808: remove overide from print_status()

MCP9808: rename uorb publisher

MCP9808: explicitly initialize temp_raw

MCP9808: explicitly initialize buffer

MCP9808: Remove enum class Register : uint8_t

MCP9808: move functions out of main + cleanup whitespace
This commit is contained in:
TedObrien
2025-03-28 21:34:48 +00:00
committed by Jacob Dahl
parent b5c3c11622
commit ddb98abf1d
3 changed files with 64 additions and 86 deletions
@@ -33,16 +33,58 @@
#include "mcp9808.h" #include "mcp9808.h"
MCP9808::MCP9808(const I2CSPIDriverConfig &config) :
I2C(config),
I2CSPIDriver(config),
_cycle_perf(perf_alloc(PC_ELAPSED, MODULE_NAME": single-sample")),
_comms_errors(perf_alloc(PC_COUNT, MODULE_NAME": comms errors"))
{
_sensor_temp.device_id = this->get_device_id();
}
MCP9808::~MCP9808()
{
ScheduleClear();
perf_free(_cycle_perf);
perf_free(_comms_errors);
}
void MCP9808::RunImpl()
{
perf_begin(_cycle_perf);
// publish at around 5HZ
if ((hrt_elapsed_time(&measurement_time)) > 200_ms) {
float temperature = read_temperature();
measurement_time = hrt_absolute_time(); // get the time the measurement was taken
if (std::isnan(temperature)) {
perf_count(_comms_errors);
} else {
_sensor_temp.timestamp = hrt_absolute_time();
_sensor_temp.timestamp_sample = measurement_time;
_sensor_temp.temperature = temperature;
_sensor_temp_pub.publish(_sensor_temp);
}
}
perf_end(_cycle_perf);
}
int MCP9808::probe() int MCP9808::probe()
{ {
uint16_t manuf_id, device_id; uint16_t manuf_id, device_id;
int ret = read_reg(Register::MANUF_ID, manuf_id); int ret = read_reg(MCP9808_REG_MANUF_ID, manuf_id);
if (ret != PX4_OK) { if (ret != PX4_OK) {
return ret; return ret;
} }
ret = read_reg(Register::DEVICE_ID, device_id); ret = read_reg(MCP9808_REG_DEVICE_ID, device_id);
if (ret != PX4_OK) { if (ret != PX4_OK) {
return ret; return ret;
@@ -69,14 +111,14 @@ int MCP9808::init()
PX4_INFO("I2C initialized successfully"); PX4_INFO("I2C initialized successfully");
ret = write_reg(Register::CONFIG, 0x0000); // Ensure default configuration ret = write_reg(MCP9808_REG_CONFIG, 0x0000); // Ensure default configuration
if (ret != PX4_OK) { if (ret != PX4_OK) {
PX4_ERR("Configuration failed"); PX4_ERR("Configuration failed");
return ret; return ret;
} }
_to_sensor_temp.advertise(); _sensor_temp_pub.advertise();
ScheduleOnInterval(100_ms); // Sample at 10 Hz ScheduleOnInterval(100_ms); // Sample at 10 Hz
return PX4_OK; return PX4_OK;
@@ -84,9 +126,9 @@ int MCP9808::init()
float MCP9808::read_temperature() float MCP9808::read_temperature()
{ {
uint16_t temp_raw; uint16_t temp_raw = 0;
if (read_reg(Register::AMBIENT_TEMP, temp_raw) != PX4_OK) { if (read_reg(MCP9808_REG_AMBIENT_TEMP, temp_raw) != PX4_OK) {
return NAN; return NAN;
} }
@@ -99,10 +141,10 @@ float MCP9808::read_temperature()
return temp; return temp;
} }
int MCP9808::read_reg(Register address, uint16_t &data) int MCP9808::read_reg(uint8_t address, uint16_t &data)
{ {
uint8_t addr = static_cast<uint8_t>(address); uint8_t addr = address;
uint8_t buffer[2]; // Buffer to hold the raw data uint8_t buffer[2] = {}; // Buffer to hold the raw data
// Read 2 bytes from the register // Read 2 bytes from the register
int ret = transfer(&addr, 1, buffer, 2); int ret = transfer(&addr, 1, buffer, 2);
@@ -117,8 +159,8 @@ int MCP9808::read_reg(Register address, uint16_t &data)
return PX4_OK; return PX4_OK;
} }
int MCP9808::write_reg(Register address, uint16_t value) int MCP9808::write_reg(uint8_t address, uint16_t value)
{ {
uint8_t buf[3] = {static_cast<uint8_t>(address), static_cast<uint8_t>(value >> 8), static_cast<uint8_t>(value & 0xFF)}; uint8_t buf[3] = {address, static_cast<uint8_t>(value >> 8), static_cast<uint8_t>(value & 0xFF)};
return transfer(buf, sizeof(buf), nullptr, 0); return transfer(buf, sizeof(buf), nullptr, 0);
} }
@@ -44,6 +44,12 @@
using namespace time_literals; using namespace time_literals;
#define MCP9808_REG_CONFIG 0x01
#define MCP9808_REG_AMBIENT_TEMP 0x05
#define MCP9808_REG_MANUF_ID 0x06
#define MCP9808_REG_DEVICE_ID 0x07
#define MCP9808_REG_RESOLUTION 0x08
class MCP9808 : public device::I2C, public I2CSPIDriver<MCP9808> class MCP9808 : public device::I2C, public I2CSPIDriver<MCP9808>
{ {
public: public:
@@ -56,30 +62,15 @@ public:
static void print_usage(); static void print_usage();
protected: protected:
void print_status() override; void print_status();
void exit_and_cleanup() override;
private: private:
enum class Register : uint8_t { uORB::PublicationMulti<sensor_temp_s> _sensor_temp_pub{ORB_ID(sensor_temp)};
CONFIG = 0x01,
AMBIENT_TEMP = 0x05,
MANUF_ID = 0x06,
DEVICE_ID = 0x07,
RESOLUTION = 0x08
};
uORB::PublicationMulti<sensor_temp_s> _to_sensor_temp{ORB_ID(sensor_temp)};
perf_counter_t _cycle_perf; perf_counter_t _cycle_perf;
perf_counter_t _comms_errors; perf_counter_t _comms_errors;
sensor_temp_s _sensor_temp{}; sensor_temp_s _sensor_temp{};
int read_reg(uint8_t address, uint16_t &data);
int write_reg(uint8_t address, uint16_t value);
int read_reg(Register address, uint16_t &data);
int write_reg(Register address, uint16_t value);
float read_temperature(); float read_temperature();
hrt_abstime measurement_time = 0;
uint32_t measurement_time = 0;
}; };
@@ -41,59 +41,6 @@
#include "mcp9808.h" #include "mcp9808.h"
#include <px4_platform_common/module.h> #include <px4_platform_common/module.h>
MCP9808::MCP9808(const I2CSPIDriverConfig &config) :
I2C(config),
I2CSPIDriver(config),
_cycle_perf(perf_alloc(PC_ELAPSED, MODULE_NAME": single-sample")),
_comms_errors(perf_alloc(PC_COUNT, MODULE_NAME": comms errors"))
{
_sensor_temp.device_id = this->get_device_id();
}
MCP9808::~MCP9808()
{
ScheduleClear();
perf_free(_cycle_perf);
perf_free(_comms_errors);
}
void MCP9808::exit_and_cleanup()
{
I2CSPIDriverBase::exit_and_cleanup(); // nothing to do
}
void MCP9808::RunImpl()
{
perf_begin(_cycle_perf);
// publish at around 5HZ
if ((hrt_absolute_time() - measurement_time) > 200_ms) {
measurement_time = hrt_absolute_time(); // get the time the measurement was taken
float temperature = read_temperature();
if (std::isnan(temperature)) {
perf_count(_comms_errors);
} else {
_sensor_temp.timestamp = hrt_absolute_time();
_sensor_temp.timestamp_sample = measurement_time;
_sensor_temp.temperature = temperature;
_to_sensor_temp.publish(_sensor_temp);
}
}
perf_end(_cycle_perf);
}
void MCP9808::print_usage() void MCP9808::print_usage()
{ {
PRINT_MODULE_USAGE_NAME("mcp9808", "driver"); PRINT_MODULE_USAGE_NAME("mcp9808", "driver");
@@ -103,13 +50,11 @@ void MCP9808::print_usage()
PRINT_MODULE_USAGE_DEFAULT_COMMANDS(); PRINT_MODULE_USAGE_DEFAULT_COMMANDS();
} }
void MCP9808::print_status() void MCP9808::print_status()
{ {
I2CSPIDriverBase::print_status(); I2CSPIDriverBase::print_status();
perf_print_counter(_cycle_perf); perf_print_counter(_cycle_perf);
perf_print_counter(_comms_errors); perf_print_counter(_comms_errors);
} }
extern "C" int mcp9808_main(int argc, char *argv[]) extern "C" int mcp9808_main(int argc, char *argv[])