refactor ist8308: use driver base class

src/drivers/magnetometer/isentek/ist8308/IST8308.cpp

@@ -40,9 +40,9 @@ static constexpr int16_t combine(uint8_t msb, uint8_t lsb)
 	return (msb << 8u) | lsb;
 }
 
-IST8308::IST8308(int bus, uint8_t address, enum Rotation rotation) :
+IST8308::IST8308(I2CSPIBusOption bus_option, int bus, enum Rotation rotation, int bus_frequency) :
-	I2C(MODULE_NAME, nullptr, bus, address, I2C_SPEED),
+	I2C(MODULE_NAME, nullptr, bus, I2C_ADDRESS_DEFAULT, bus_frequency),
-	ScheduledWorkItem(MODULE_NAME, px4::device_bus_to_wq(get_device_id())),
+	I2CSPIDriver(MODULE_NAME, px4::device_bus_to_wq(get_device_id()), bus_option, bus),
 	_px4_mag(get_device_id(), external() ? ORB_PRIO_VERY_HIGH : ORB_PRIO_HIGH, rotation)
 {
 	set_device_type(DRV_MAG_DEVTYPE_IST8308);
@@ -53,43 +53,34 @@ IST8308::IST8308(int bus, uint8_t address, enum Rotation rotation) :
 
 IST8308::~IST8308()
 {
-	Stop();
-
 	perf_free(_transfer_perf);
 	perf_free(_bad_register_perf);
 	perf_free(_bad_transfer_perf);
 }
 
-bool IST8308::Init()
+int IST8308::init()
 {
-	if (I2C::init() != PX4_OK) {
+	int ret = I2C::init();
-		PX4_ERR("I2C::init failed");
+
-		return false;
+	if (ret != PX4_OK) {
+		DEVICE_DEBUG("I2C::init failed (%i)", ret);
+		return ret;
 	}
 
-	return Reset();
+	return Reset() ? 0 : -1;
-}
-
-void IST8308::Stop()
-{
-	// wait until stopped
-	while (_state.load() != STATE::STOPPED) {
-		_state.store(STATE::REQUEST_STOP);
-		ScheduleNow();
-		px4_usleep(10);
-	}
 }
 
 bool IST8308::Reset()
 {
-	_state.store(STATE::RESET);
+	_state = STATE::RESET;
 	ScheduleClear();
 	ScheduleNow();
 	return true;
 }
 
-void IST8308::PrintInfo()
+void IST8308::print_status()
 {
+	I2CSPIDriverBase::print_status();
 	perf_print_counter(_transfer_perf);
 	perf_print_counter(_bad_register_perf);
 	perf_print_counter(_bad_transfer_perf);
@@ -102,21 +93,21 @@ int IST8308::probe()
 	const uint8_t whoami = RegisterRead(Register::WAI);
 
 	if (whoami != Device_ID) {
-		PX4_WARN("unexpected WAI 0x%02x", whoami);
+		DEVICE_DEBUG("unexpected WAI 0x%02x", whoami);
 		return PX4_ERROR;
 	}
 
 	return PX4_OK;
 }
 
-void IST8308::Run()
+void IST8308::RunImpl()
 {
-	switch (_state.load()) {
+	switch (_state) {
 	case STATE::RESET:
 		// CNTL3: Software Reset
 		RegisterSetAndClearBits(Register::CNTL3, CNTL3_BIT::SRST, 0);
 		_reset_timestamp = hrt_absolute_time();
-		_state.store(STATE::WAIT_FOR_RESET);
+		_state = STATE::WAIT_FOR_RESET;
 		ScheduleDelayed(50_ms); // Power On Reset: max:50ms
 		break;
 
@@ -127,14 +118,14 @@ void IST8308::Run()
 		    && ((RegisterRead(Register::CNTL3) & CNTL3_BIT::SRST) == 0)) {
 
 			// if reset succeeded then configure
-			_state.store(STATE::CONFIGURE);
+			_state = STATE::CONFIGURE;
 			ScheduleNow();
 
 		} else {
 			// RESET not complete
 			if (hrt_elapsed_time(&_reset_timestamp) > 100_ms) {
 				PX4_ERR("Reset failed, retrying");
-				_state.store(STATE::RESET);
+				_state = STATE::RESET;
 				ScheduleNow();
 
 			} else {
@@ -148,7 +139,7 @@ void IST8308::Run()
 	case STATE::CONFIGURE:
 		if (Configure()) {
 			// if configure succeeded then start reading every 20 ms (50 Hz)
-			_state.store(STATE::READ);
+			_state = STATE::READ;
 			ScheduleOnInterval(20_ms, 20_ms);
 
 		} else {
@@ -203,22 +194,13 @@ void IST8308::Run()
 				} else {
 					// register check failed, force reconfigure
 					PX4_DEBUG("Health check failed, reconfiguring");
-					_state.store(STATE::CONFIGURE);
+					_state = STATE::CONFIGURE;
 					ScheduleNow();
 				}
 			}
 		}
 
 		break;
-
-	case STATE::REQUEST_STOP:
-		ScheduleClear();
-		_state.store(STATE::STOPPED);
-		break;
-
-	case STATE::STOPPED:
-		// DO NOTHING
-		break;
 	}
 }
 

src/drivers/magnetometer/isentek/ist8308/IST8308.hpp

@@ -47,22 +47,31 @@
 #include <lib/drivers/magnetometer/PX4Magnetometer.hpp>
 #include <lib/perf/perf_counter.h>
 #include <px4_platform_common/atomic.h>
-#include <px4_platform_common/px4_work_queue/ScheduledWorkItem.hpp>
+#include <px4_platform_common/i2c_spi_buses.h>
 
 using namespace iSentek_IST8308;
 
-class IST8308 : public device::I2C, public px4::ScheduledWorkItem
+class IST8308 : public device::I2C, public I2CSPIDriver<IST8308>
 {
 public:
-	IST8308(int bus, uint8_t address = I2C_ADDRESS_DEFAULT, enum Rotation rotation = ROTATION_NONE);
+	IST8308(I2CSPIBusOption bus_option, int bus, enum Rotation rotation, int bus_frequency);
 	~IST8308() override;
 
-	bool Init();
+	static I2CSPIDriverBase *instantiate(const BusCLIArguments &cli, const BusInstanceIterator &iterator,
-	void Start();
+					     int runtime_instance);
-	void Stop();
+	static void print_usage();
-	bool Reset();
-	void PrintInfo();
 
+	void print_status();
+
+	int init() override;
+
+	void Start();
+	bool Reset();
+
+	void     RunImpl();
+
+protected:
+	void custom_method(const BusCLIArguments &cli) override;
 private:
 
 	// Sensor Configuration
@@ -74,8 +83,6 @@ private:
 
 	int probe() override;
 
-	void Run() override;
-
 	bool Configure();
 
 	bool RegisterCheck(const register_config_t &reg_cfg, bool notify = false);
@@ -100,11 +107,9 @@ private:
 		WAIT_FOR_RESET,
 		CONFIGURE,
 		READ,
-		REQUEST_STOP,
-		STOPPED,
 	};
 
-	px4::atomic<STATE> _state{STATE::RESET};
+	STATE _state{STATE::RESET};
 
 	static constexpr uint8_t size_register_cfg{5};
 	register_config_t _register_cfg[size_register_cfg] {

src/drivers/magnetometer/isentek/ist8308/ist8308_main.cpp

@@ -34,177 +34,83 @@
 #include "IST8308.hpp"
 
 #include <px4_platform_common/getopt.h>
+#include <px4_platform_common/module.h>
 
-namespace ist8308
+void
+IST8308::print_usage()
 {
-
+	PRINT_MODULE_USAGE_NAME("ist8308", "driver");
-enum class BUS {
+	PRINT_MODULE_USAGE_SUBCATEGORY("magnetometer");
-	ALL = 0,
+	PRINT_MODULE_USAGE_COMMAND("start");
-	I2C_EXTERNAL  = 1,
+	PRINT_MODULE_USAGE_PARAMS_I2C_SPI_DRIVER(true, false);
-	I2C_EXTERNAL1 = 2,
+	PRINT_MODULE_USAGE_PARAM_INT('R', 0, 0, 35, "Rotation", true);
-	I2C_EXTERNAL2 = 3,
+	PRINT_MODULE_USAGE_COMMAND("reset");
-	I2C_EXTERNAL3 = 4,
+	PRINT_MODULE_USAGE_DEFAULT_COMMANDS();
-	I2C_INTERNAL  = 5,
-};
-
-struct bus_option {
-	BUS busid;
-	uint8_t busnum;
-	IST8308 *dev;
-} bus_options[] = {
-#if defined(PX4_I2C_BUS_ONBOARD)
-	{ BUS::I2C_INTERNAL, PX4_I2C_BUS_ONBOARD, nullptr },
-#endif
-#if defined(PX4_I2C_BUS_EXPANSION)
-	{ BUS::I2C_EXTERNAL, PX4_I2C_BUS_EXPANSION, nullptr },
-#endif
-#if defined(PX4_I2C_BUS_EXPANSION1)
-	{ BUS::I2C_EXTERNAL1, PX4_I2C_BUS_EXPANSION1, nullptr },
-#endif
-#if defined(PX4_I2C_BUS_EXPANSION2)
-	{ BUS::I2C_EXTERNAL2, PX4_I2C_BUS_EXPANSION2, nullptr },
-#endif
-#if defined(PX4_I2C_BUS_EXPANSION3)
-	{ BUS::I2C_EXTERNAL3, PX4_I2C_BUS_EXPANSION3, nullptr },
-#endif
-};
-
-// find a bus structure for a busid
-static bus_option *find_bus(BUS busid)
-{
-	for (bus_option &bus_option : bus_options) {
-		if ((busid == BUS::ALL || busid == bus_option.busid)
-		    && bus_option.dev != nullptr) {
-
-			return &bus_option;
-		}
-	}
-
-	return nullptr;
 }
 
-static bool start_bus(bus_option &bus, enum Rotation rotation)
+I2CSPIDriverBase *IST8308::instantiate(const BusCLIArguments &cli, const BusInstanceIterator &iterator,
+				       int runtime_instance)
 {
-	IST8308 *dev = new IST8308(bus.busnum, I2C_ADDRESS_DEFAULT, rotation);
+	IST8308 *instance = new IST8308(iterator.configuredBusOption(), iterator.bus(), cli.rotation, cli.bus_frequency);
 
-	if (dev == nullptr) {
+	if (!instance) {
 		PX4_ERR("alloc failed");
-		return false;
+		return nullptr;
 	}
 
-	if (!dev->Init()) {
+	if (OK != instance->init()) {
-		PX4_ERR("driver start failed");
+		delete instance;
-		delete dev;
+		return nullptr;
-		return false;
 	}
 
-	bus.dev = dev;
+	return instance;
-
-	return true;
 }
 
-static int start(BUS busid, enum Rotation rotation)
+void IST8308::custom_method(const BusCLIArguments &cli)
 {
-	for (bus_option &bus_option : bus_options) {
+	Reset();
-		if (bus_option.dev != nullptr) {
-			// this device is already started
-			PX4_WARN("already started");
-			continue;
-		}
-
-		if (busid != BUS::ALL && bus_option.busid != busid) {
-			// not the one that is asked for
-			continue;
-		}
-
-		if (start_bus(bus_option, rotation)) {
-			return 0;
-		}
-	}
-
-	return -1;
 }
 
-static int stop(BUS busid)
-{
-	bus_option *bus = find_bus(busid);
-
-	if (bus != nullptr && bus->dev != nullptr) {
-		delete bus->dev;
-		bus->dev = nullptr;
-
-	} else {
-		PX4_WARN("driver not running");
-		return -1;
-	}
-
-	return 0;
-}
-
-static int status(BUS busid)
-{
-	bus_option *bus = find_bus(busid);
-
-	if (bus != nullptr && bus->dev != nullptr) {
-		bus->dev->PrintInfo();
-		return 0;
-	}
-
-	PX4_WARN("driver not running");
-	return -1;
-}
-
-static int usage()
-{
-	PX4_INFO("missing command: try 'start', 'stop', 'status'");
-	PX4_INFO("options:");
-	PX4_INFO("    -R rotation");
-
-	return 0;
-}
-
-} // namespace ist8308
-
 extern "C" int ist8308_main(int argc, char *argv[])
 {
-	enum Rotation rotation = ROTATION_NONE;
+	int ch;
-	int myoptind = 1;
+	using ThisDriver = IST8308;
-	int ch = 0;
+	BusCLIArguments cli{true, false};
-	const char *myoptarg = nullptr;
+	cli.default_i2c_frequency = I2C_SPEED;
-	ist8308::BUS busid = ist8308::BUS::ALL;
 
-	// start options
+	while ((ch = cli.getopt(argc, argv, "R:")) != EOF) {
-	while ((ch = px4_getopt(argc, argv, "R:b:", &myoptind, &myoptarg)) != EOF) {
 		switch (ch) {
 		case 'R':
-			rotation = (enum Rotation)atoi(myoptarg);
+			cli.rotation = (enum Rotation)atoi(cli.optarg());
 			break;
-
-		case 'b':
-			busid = (ist8308::BUS)atoi(myoptarg);
-			break;
-
-		default:
-			return ist8308::usage();
 		}
 	}
 
-	if (myoptind >= argc) {
+	const char *verb = cli.optarg();
-		return ist8308::usage();
+
+	if (!verb) {
+		ThisDriver::print_usage();
+		return -1;
 	}
 
-	const char *verb = argv[myoptind];
+	BusInstanceIterator iterator(MODULE_NAME, cli, DRV_MAG_DEVTYPE_IST8308);
 
 	if (!strcmp(verb, "start")) {
-		return ist8308::start(busid, rotation);
+		return ThisDriver::module_start(cli, iterator);
-
-	} else if (!strcmp(verb, "stop")) {
-		return ist8308::stop(busid);
-
-	} else if (!strcmp(verb, "status")) {
-		return ist8308::status(busid);
-
 	}
 
-	return ist8308::usage();
+	if (!strcmp(verb, "stop")) {
+		return ThisDriver::module_stop(iterator);
+	}
+
+	if (!strcmp(verb, "status")) {
+		return ThisDriver::module_status(iterator);
+	}
+
+	if (!strcmp(verb, "reset")) {
+		return ThisDriver::module_custom_method(cli, iterator);
+	}
+
+	ThisDriver::print_usage();
+	return -1;
 }