bmp388: cleanup main and add to px4_fmu-v5x board_config.h

boards/px4/fmu-v5x/src/board_config.h

@@ -276,6 +276,13 @@
 #define PX4_I2C_BUS_ONBOARD         4
 #define PX4_I2C_BUS_LED             PX4_I2C_BUS_EXPANSION
 
+/* Devices on the onboard bus.
+ *
+ * Note that these are unshifted addresses.
+ */
+#define PX4_I2C_OBDEV_BMP388        0x76
+#define PX4_I2C_OBDEV_A71CH         0x49
+
 #define BOARD_NUMBER_I2C_BUSES      4
 #define BOARD_I2C_BUS_CLOCK_INIT    {100000, 100000, 100000, 100000}
 

src/drivers/barometer/bmp388/bmp388.cpp

@@ -42,19 +42,13 @@
 
 #include "bmp388.h"
 
-BMP388::BMP388(IBMP388 *interface, const char *path) :
+BMP388::BMP388(IBMP388 *interface) :
-	CDev(path),
 	ScheduledWorkItem(MODULE_NAME, px4::device_bus_to_wq(interface->get_device_id())),
-	_px4_baro(interface->get_device_id(), ORB_PRIO_DEFAULT),
+	_px4_baro(interface->get_device_id()),
 	_interface(interface),
-	_osr_t(BMP3_OVERSAMPLING_2X),
+	_sample_perf(perf_alloc(PC_ELAPSED, MODULE_NAME": read")),
-	_osr_p(BMP3_OVERSAMPLING_16X),
+	_measure_perf(perf_alloc(PC_ELAPSED, MODULE_NAME": measure")),
-	_odr(BMP3_ODR_50_HZ),
+	_comms_errors(perf_alloc(PC_COUNT, MODULE_NAME": comms errors"))
-	_iir_coef(BMP3_IIR_FILTER_DISABLE),
-	_sample_perf(perf_alloc(PC_ELAPSED, "bmp388: read")),
-	_measure_perf(perf_alloc(PC_ELAPSED, "bmp388: measure")),
-	_comms_errors(perf_alloc(PC_COUNT, "bmp388: comms errors")),
-	_collect_phase(false)
 {
 	_px4_baro.set_device_type(DRV_DEVTYPE_BMP388);
 }
@@ -75,13 +69,6 @@ BMP388::~BMP388()
 int
 BMP388::init()
 {
-	int ret = CDev::init();
-
-	if (ret != OK) {
-		PX4_ERR("CDev init failed");
-		return ret;
-	}
-
 	if (!soft_reset()) {
 		PX4_WARN("failed to reset baro during init");
 		return -EIO;
@@ -111,6 +98,7 @@ BMP388::init()
 
 	/* do a first measurement cycle to populate reports with valid data */
 	if (measure()) {
+		PX4_WARN("measure failed");
 		return -EIO;
 	}
 
@@ -118,6 +106,7 @@ BMP388::init()
 	px4_usleep(_measure_interval);
 
 	if (collect()) {
+		PX4_WARN("collect failed");
 		return -EIO;
 	}
 

src/drivers/barometer/bmp388/bmp388.h

@@ -281,14 +281,11 @@ struct fcalibration_s {
 /*
  * BMP388 internal constants and data structures.
  */
-
-
 class IBMP388
 {
 public:
 	virtual ~IBMP388() = default;
 
-	virtual bool is_external() = 0;
 	virtual int init() = 0;
 
 	// read reg value
@@ -307,39 +304,35 @@ public:
 	virtual calibration_s *get_calibration(uint8_t addr) = 0;
 
 	virtual uint32_t get_device_id() const = 0;
-
 };
 
-class BMP388 : public cdev::CDev, public px4::ScheduledWorkItem
+class BMP388 : public px4::ScheduledWorkItem
 {
 public:
-	BMP388(IBMP388 *interface, const char *path);
+	BMP388(IBMP388 *interface);
 	virtual ~BMP388();
 
 	virtual int		init();
 
-	/**
-	 * Diagnostics - print some basic information about the driver.
-	 */
 	void			print_info();
 
 private:
 	PX4Barometer		_px4_baro;
-	IBMP388			*_interface;
+	IBMP388			*_interface{nullptr};
 
 	unsigned		_measure_interval{0};			// interval in microseconds needed to measure
-	uint8_t			_osr_t;               // oversampling rate, temperature
+	uint8_t			_osr_t{BMP3_OVERSAMPLING_2X};		// oversampling rate, temperature
-	uint8_t			_osr_p;               // oversampling rate, pressure
+	uint8_t			_osr_p{BMP3_OVERSAMPLING_16X};		// oversampling rate, pressure
-	uint8_t			_odr;                 // output data rate
+	uint8_t			_odr{BMP3_ODR_50_HZ};			// output data rate
-	uint8_t			_iir_coef;            // IIR coefficient
+	uint8_t			_iir_coef{BMP3_IIR_FILTER_DISABLE};	// IIR coefficient
 
 	perf_counter_t		_sample_perf;
 	perf_counter_t		_measure_perf;
 	perf_counter_t		_comms_errors;
 
-	struct calibration_s 	*_cal; // stored calibration constants
+	calibration_s		*_cal {nullptr}; // stored calibration constants
 
-	bool			_collect_phase;
+	bool			_collect_phase{false};
 
 	void 			Run() override;
 	void 			start();
@@ -360,6 +353,6 @@ private:
 
 
 /* interface factories */
-extern IBMP388 *bmp388_spi_interface(uint8_t busnum, uint32_t device, bool external);
+extern IBMP388 *bmp388_spi_interface(uint8_t busnum, uint32_t device);
-extern IBMP388 *bmp388_i2c_interface(uint8_t busnum, uint32_t device, bool external);
+extern IBMP388 *bmp388_i2c_interface(uint8_t busnum, uint32_t device);
-typedef IBMP388 *(*BMP388_constructor)(uint8_t, uint32_t, bool);
+typedef IBMP388 *(*BMP388_constructor)(uint8_t, uint32_t);

src/drivers/barometer/bmp388/bmp388_i2c.cpp

@@ -46,10 +46,9 @@
 class BMP388_I2C: public device::I2C, public IBMP388
 {
 public:
-	BMP388_I2C(uint8_t bus, uint32_t device, bool external);
+	BMP388_I2C(uint8_t bus, uint32_t device);
 	virtual ~BMP388_I2C() = default;
 
-	bool is_external();
 	int init();
 
 	uint8_t get_reg(uint8_t addr);
@@ -63,23 +62,16 @@ public:
 private:
 	struct calibration_s _cal;
 	struct data_s _data;
-	bool _external;
 };
 
-IBMP388 *bmp388_i2c_interface(uint8_t busnum, uint32_t device, bool external)
+IBMP388 *bmp388_i2c_interface(uint8_t busnum, uint32_t device)
 {
-	return new BMP388_I2C(busnum, device, external);
+	return new BMP388_I2C(busnum, device);
 }
 
-BMP388_I2C::BMP388_I2C(uint8_t bus, uint32_t device, bool external) :
+BMP388_I2C::BMP388_I2C(uint8_t bus, uint32_t device) :
 	I2C("BMP388_I2C", nullptr, bus, device, 100 * 1000)
 {
-	_external = external;
-}
-
-bool BMP388_I2C::is_external()
-{
-	return _external;
 }
 
 int BMP388_I2C::init()

src/drivers/barometer/bmp388/bmp388_main.cpp

@@ -31,216 +31,205 @@
  *
  ****************************************************************************/
 
+#include <px4_platform_common/px4_config.h>
 #include <px4_platform_common/getopt.h>
 
 #include "bmp388.h"
 
-enum BMP388_BUS {
+enum class BMP388_BUS {
-	BMP388_BUS_ALL = 0,
+	ALL = 0,
-	BMP388_BUS_I2C_INTERNAL,
+	I2C_INTERNAL,
-	BMP388_BUS_I2C_INTERNAL1,
+	I2C_INTERNAL1,
-	BMP388_BUS_I2C_EXTERNAL,
+	I2C_EXTERNAL,
-	BMP388_BUS_SPI_INTERNAL,
+	SPI_INTERNAL,
-	BMP388_BUS_SPI_EXTERNAL
+	SPI_EXTERNAL
 };
 
-/**
- * Local functions in support of the shell command.
- */
 namespace bmp388
 {
 
-/*
+// list of supported bus configurations
- * list of supported bus configurations
- */
 struct bmp388_bus_option {
-	enum BMP388_BUS busid;
+	BMP388_BUS busid;
-	const char *devpath;
 	BMP388_constructor interface_constructor;
 	uint8_t busnum;
-	uint32_t device;
+	uint32_t address;
-	bool external;
 	BMP388 *dev;
 } bus_options[] = {
 #if defined(PX4_SPIDEV_EXT_BARO) && defined(PX4_SPI_BUS_EXT)
-	{ BMP388_BUS_SPI_EXTERNAL, "/dev/bmp388_spi_ext", &bmp388_spi_interface, PX4_SPI_BUS_EXT, PX4_SPIDEV_EXT_BARO, true, NULL },
+	{ BMP388_BUS::SPI_EXTERNAL, &bmp388_spi_interface, PX4_SPI_BUS_EXT, PX4_SPIDEV_EXT_BARO, nullptr },
 #endif
 #if defined(PX4_SPIDEV_BARO)
 #  if defined(PX4_SPIDEV_BARO_BUS)
-	{ BMP388_BUS_SPI_INTERNAL, "/dev/bmp388_spi_int", &bmp388_spi_interface, PX4_SPIDEV_BARO_BUS, PX4_SPIDEV_BARO, false, NULL },
+	{ BMP388_BUS::SPI_INTERNAL, &bmp388_spi_interface, PX4_SPIDEV_BARO_BUS, PX4_SPIDEV_BARO, nullptr },
 #  else
-	{ BMP388_BUS_SPI_INTERNAL, "/dev/bmp388_spi_int", &bmp388_spi_interface, PX4_SPI_BUS_SENSORS, PX4_SPIDEV_BARO, false, NULL },
+	{ BMP388_BUS::SPI_INTERNAL, &bmp388_spi_interface, PX4_SPI_BUS_SENSORS, PX4_SPIDEV_BARO, nullptr },
 #  endif
 #endif
 #if defined(PX4_I2C_BUS_ONBOARD) && defined(PX4_I2C_OBDEV_BMP388)
-	{ BMP388_BUS_I2C_INTERNAL, "/dev/bmp388_i2c_int", &bmp388_i2c_interface, PX4_I2C_BUS_ONBOARD, PX4_I2C_OBDEV_BMP388, false, NULL },
+	{ BMP388_BUS::I2C_INTERNAL, &bmp388_i2c_interface, PX4_I2C_BUS_ONBOARD, PX4_I2C_OBDEV_BMP388, nullptr },
 #endif
 #if defined(PX4_I2C_BUS_ONBOARD) && defined(PX4_I2C_OBDEV1_BMP388)
-	{ BMP388_BUS_I2C_INTERNAL1, "/dev/bmp388_i2c_int1", &bmp388_i2c_interface, PX4_I2C_BUS_ONBOARD, PX4_I2C_OBDEV1_BMP388, false, NULL },
+	{ BMP388_BUS::I2C_INTERNAL1, &bmp388_i2c_interface, PX4_I2C_BUS_ONBOARD, PX4_I2C_OBDEV1_BMP388, nullptr },
 #endif
 #if defined(PX4_I2C_BUS_EXPANSION) && defined(PX4_I2C_OBDEV_BMP388)
-	{ BMP388_BUS_I2C_EXTERNAL, "/dev/bmp388_i2c_ext", &bmp388_i2c_interface, PX4_I2C_BUS_EXPANSION, PX4_I2C_OBDEV_BMP388, true, NULL },
+	{ BMP388_BUS::I2C_EXTERNAL, &bmp388_i2c_interface, PX4_I2C_BUS_EXPANSION, PX4_I2C_OBDEV_BMP388, nullptr },
 #endif
 };
-#define NUM_BUS_OPTIONS (sizeof(bus_options)/sizeof(bus_options[0]))
 
-/**
+// find a bus structure for a busid
- * Start the driver.
+static struct bmp388_bus_option *find_bus(BMP388_BUS busid)
- */
-bool
-start_bus(struct bmp388_bus_option &bus)
 {
-	if (bus.dev != nullptr) {
+	for (bmp388_bus_option &bus_option : bus_options) {
-		PX4_ERR("bus option already started");
+		if ((busid == BMP388_BUS::ALL ||
-		exit(1);
+		     busid == bus_option.busid) && bus_option.dev != nullptr) {
+
+			return &bus_option;
+		}
 	}
 
-	IBMP388 *interface = bus.interface_constructor(bus.busnum, bus.device, bus.external);
+	return nullptr;
+}
 
-	if (interface->init() != OK) {
+static bool start_bus(bmp388_bus_option &bus)
-		delete interface;
+{
+	IBMP388 *interface = bus.interface_constructor(bus.busnum, bus.address);
+
+	if ((interface == nullptr) || (interface->init() != PX4_OK)) {
 		PX4_WARN("no device on bus %u", (unsigned)bus.busid);
+		delete interface;
 		return false;
 	}
 
-	bus.dev = new BMP388(interface, bus.devpath);
+	BMP388 *dev = new BMP388(interface);
 
-	if (bus.dev == nullptr) {
+	if (dev == nullptr || (dev->init() != PX4_OK)) {
+		PX4_ERR("driver start failed");
+		delete dev;
+		delete interface;
 		return false;
 	}
 
-	if (OK != bus.dev->init()) {
+	bus.dev = dev;
-		delete bus.dev;
-		bus.dev = nullptr;
-		return false;
-	}
 
 	return true;
 }
 
-/**
+static int start(BMP388_BUS busid)
- * Start the driver.
- *
- * This function call only returns once the driver
- * is either successfully up and running or failed to start.
- */
-void
-start(enum BMP388_BUS busid)
 {
-	uint8_t i;
+	for (bmp388_bus_option &bus_option : bus_options) {
-	bool started = false;
+		if (bus_option.dev != nullptr) {
-
-	for (i = 0; i < NUM_BUS_OPTIONS; i++) {
-		if (busid == BMP388_BUS_ALL && bus_options[i].dev != NULL) {
 			// this device is already started
+			PX4_WARN("already started");
 			continue;
 		}
 
-		if (busid != BMP388_BUS_ALL && bus_options[i].busid != busid) {
+		if (busid != BMP388_BUS::ALL && bus_option.busid != busid) {
 			// not the one that is asked for
 			continue;
 		}
 
-		started |= start_bus(bus_options[i]);
+		if (start_bus(bus_option)) {
+			return PX4_OK;
+		}
 	}
 
-	if (!started) {
+	return PX4_ERROR;
-		PX4_WARN("bus option number is %d", i);
-		PX4_ERR("driver start failed");
-		exit(1);
 }
 
-	// one or more drivers started OK
+static int stop(BMP388_BUS busid)
-	exit(0);
-}
-
-/**
- * Print a little info about the driver.
- */
-void
-info()
 {
-	for (uint8_t i = 0; i < NUM_BUS_OPTIONS; i++) {
+	bmp388_bus_option *bus = find_bus(busid);
-		struct bmp388_bus_option &bus = bus_options[i];
 
-		if (bus.dev != nullptr) {
+	if (bus != nullptr && bus->dev != nullptr) {
-			PX4_WARN("%s", bus.devpath);
+		delete bus->dev;
-			bus.dev->print_info();
+		bus->dev = nullptr;
-		}
+
+	} else {
+		PX4_WARN("driver not running");
+		return PX4_ERROR;
 	}
 
-	exit(0);
+	return PX4_OK;
 }
 
-void
+static int status(BMP388_BUS busid)
-usage()
 {
-	PX4_WARN("missing command: try 'start', 'info'");
+	bmp388_bus_option *bus = find_bus(busid);
-	PX4_WARN("options:");
+
-	PX4_WARN("    -X    (external I2C bus TODO)");
+	if (bus != nullptr && bus->dev != nullptr) {
-	PX4_WARN("    -I    (internal I2C bus TODO)");
+		bus->dev->print_info();
-	PX4_WARN("    -S    (external SPI bus)");
+		return PX4_OK;
-	PX4_WARN("    -s    (internal SPI bus)");
+	}
+
+	PX4_WARN("driver not running");
+	return PX4_ERROR;
+}
+
+static int usage()
+{
+	PX4_INFO("missing command: try 'start', 'stop', 'status'");
+	PX4_INFO("options:");
+	PX4_INFO("    -X    (i2c external bus)");
+	PX4_INFO("    -I    (i2c internal bus)");
+	PX4_INFO("    -J    (i2c internal bus 2)");
+	PX4_INFO("    -s    (spi internal bus)");
+	PX4_INFO("    -S    (spi external bus)");
+
+	return 0;
 }
 
 } // namespace
 
-extern "C" __EXPORT int bmp388_main(int argc, char *argv[])
+extern "C" int bmp388_main(int argc, char *argv[])
 {
 	int myoptind = 1;
 	int ch;
 	const char *myoptarg = nullptr;
-	enum BMP388_BUS busid = BMP388_BUS_ALL;
+	BMP388_BUS busid = BMP388_BUS::ALL;
 
 	while ((ch = px4_getopt(argc, argv, "XIJSs", &myoptind, &myoptarg)) != EOF) {
 		switch (ch) {
 		case 'X':
-			busid = BMP388_BUS_I2C_EXTERNAL;
+			busid = BMP388_BUS::I2C_EXTERNAL;
 			break;
 
 		case 'I':
-			busid = BMP388_BUS_I2C_INTERNAL;
+			busid = BMP388_BUS::I2C_INTERNAL;
 			break;
 
 		case 'J':
-			busid = BMP388_BUS_I2C_INTERNAL1;
+			busid = BMP388_BUS::I2C_INTERNAL1;
 			break;
 
 		case 'S':
-			busid = BMP388_BUS_SPI_EXTERNAL;
+			busid = BMP388_BUS::SPI_EXTERNAL;
 			break;
 
 		case 's':
-			busid = BMP388_BUS_SPI_INTERNAL;
+			busid = BMP388_BUS::SPI_INTERNAL;
 			break;
 
 		default:
-			bmp388::usage();
+			return bmp388::usage();
-			return 0;
 		}
 	}
 
 	if (myoptind >= argc) {
-		bmp388::usage();
+		return bmp388::usage();
-		return -1;
 	}
 
 	const char *verb = argv[myoptind];
 
-	/*
-	 * Start/load the driver.
-	 */
 	if (!strcmp(verb, "start")) {
-		bmp388::start(busid);
+		return bmp388::start(busid);
+
+	} else if (!strcmp(verb, "stop")) {
+		return bmp388::stop(busid);
+
+	} else if (!strcmp(verb, "status")) {
+		return bmp388::status(busid);
 	}
 
-	/*
+	return bmp388::usage();
-	 * Print driver information.
-	 */
-	if (!strcmp(verb, "info")) {
-		bmp388::info();
-	}
-
-	PX4_ERR("unrecognized command, try 'start' or 'info'");
-	return -1;
 }

src/drivers/barometer/bmp388/bmp388_spi.cpp

@@ -63,10 +63,9 @@ struct spi_calibration_s {
 class BMP388_SPI: public device::SPI, public IBMP388
 {
 public:
-	BMP388_SPI(uint8_t bus, uint32_t device, bool is_external_device);
+	BMP388_SPI(uint8_t bus, uint32_t device);
 	virtual ~BMP388_SPI() = default;
 
-	bool is_external();
 	int init();
 
 	uint8_t get_reg(uint8_t addr);
@@ -80,25 +79,18 @@ public:
 private:
 	spi_calibration_s _cal;
 	spi_data_s _data;
-	bool _external;
 };
 
-IBMP388 *bmp388_spi_interface(uint8_t busnum, uint32_t device, bool external)
+IBMP388 *bmp388_spi_interface(uint8_t busnum, uint32_t device)
 {
-	return new BMP388_SPI(busnum, device, external);
+	return new BMP388_SPI(busnum, device);
 }
 
-BMP388_SPI::BMP388_SPI(uint8_t bus, uint32_t device, bool is_external_device) :
+BMP388_SPI::BMP388_SPI(uint8_t bus, uint32_t device) :
 	SPI("BMP388_SPI", nullptr, bus, device, SPIDEV_MODE3, 10 * 1000 * 1000)
 {
-	_external = is_external_device;
 }
 
-bool BMP388_SPI::is_external()
-{
-	return _external;
-};
-
 int BMP388_SPI::init()
 {
 	return SPI::init();

src/lib/drivers/barometer/PX4Barometer.hpp

@@ -45,7 +45,7 @@ class PX4Barometer : public cdev::CDev
 {
 
 public:
-	PX4Barometer(uint32_t device_id, uint8_t priority);
+	PX4Barometer(uint32_t device_id, uint8_t priority = ORB_PRIO_DEFAULT);
 	~PX4Barometer() override;
 
 	void set_device_type(uint8_t devtype);