diff --git a/boards/modalai/fc-v1/src/board_config.h b/boards/modalai/fc-v1/src/board_config.h index 6d071b1302..0b53270f99 100644 --- a/boards/modalai/fc-v1/src/board_config.h +++ b/boards/modalai/fc-v1/src/board_config.h @@ -208,7 +208,6 @@ * * Note that these are unshifted addresses. */ -#define PX4_I2C_OBDEV_BMP388 0x76 #define PX4_I2C_OBDEV_A71CH 0x49 #define BOARD_NUMBER_I2C_BUSES 4 diff --git a/boards/px4/fmu-v5x/init/rc.board_sensors b/boards/px4/fmu-v5x/init/rc.board_sensors index 6b9b468aa9..a525dbe8c6 100644 --- a/boards/px4/fmu-v5x/init/rc.board_sensors +++ b/boards/px4/fmu-v5x/init/rc.board_sensors @@ -31,7 +31,7 @@ bmm150 start # Possible internal Baro bmp388 -I start -bmp388 -J start +bmp388 -I -a 0x77 start # Baro on I2C3 ms5611 -X start diff --git a/boards/px4/fmu-v5x/src/board_config.h b/boards/px4/fmu-v5x/src/board_config.h index 43d57b6df8..be76cb5986 100644 --- a/boards/px4/fmu-v5x/src/board_config.h +++ b/boards/px4/fmu-v5x/src/board_config.h @@ -258,8 +258,6 @@ * * Note that these are unshifted addresses. */ -#define PX4_I2C_OBDEV_BMP388 0x76 // On IMU -#define PX4_I2C_OBDEV1_BMP388 0x77 // On FMUM #define PX4_I2C_OBDEV_A71CH 0x49 #define BOARD_NUMBER_I2C_BUSES 4 diff --git a/src/drivers/barometer/bmp388/bmp388.cpp b/src/drivers/barometer/bmp388/bmp388.cpp index 183e4386eb..2e5cb0f33f 100644 --- a/src/drivers/barometer/bmp388/bmp388.cpp +++ b/src/drivers/barometer/bmp388/bmp388.cpp @@ -34,16 +34,16 @@ /** * @file bmp388.cpp * - * Driver for the BMP388 barometric pressure sensor connected via I2C - * (SPI still TODO/test). + * Driver for the BMP388 barometric pressure sensor connected via SPI or I2C * * Refer to: https://github.com/BoschSensortec/BMP3-Sensor-API */ #include "bmp388.h" -BMP388::BMP388(IBMP388 *interface) : - ScheduledWorkItem(MODULE_NAME, px4::device_bus_to_wq(interface->get_device_id())), +BMP388::BMP388(I2CSPIBusOption bus_option, int bus, IBMP388 *interface) : + I2CSPIDriver(MODULE_NAME, px4::device_bus_to_wq(interface->get_device_id()), bus_option, bus, + interface->get_device_address()), _px4_baro(interface->get_device_id()), _interface(interface), _sample_perf(perf_alloc(PC_ELAPSED, MODULE_NAME": read")), @@ -55,9 +55,6 @@ BMP388::BMP388(IBMP388 *interface) : BMP388::~BMP388() { - /* make sure we are truly inactive */ - stop(); - /* free perf counters */ perf_free(_sample_perf); perf_free(_measure_perf); @@ -116,8 +113,9 @@ BMP388::init() } void -BMP388::print_info() +BMP388::print_status() { + I2CSPIDriverBase::print_status(); perf_print_counter(_sample_perf); perf_print_counter(_measure_perf); perf_print_counter(_comms_errors); @@ -131,23 +129,11 @@ BMP388::start() { _collect_phase = false; - /* make sure we are stopped first */ - stop(); - ScheduleOnInterval(_measure_interval, 1000); } void -BMP388::stop() -{ - ScheduleClear(); -} - -/* - * ScheduledWorkItem override - */ -void -BMP388::Run() +BMP388::RunImpl() { if (_collect_phase) { collect(); diff --git a/src/drivers/barometer/bmp388/bmp388.h b/src/drivers/barometer/bmp388/bmp388.h index f092e2260c..dd92a7afd5 100644 --- a/src/drivers/barometer/bmp388/bmp388.h +++ b/src/drivers/barometer/bmp388/bmp388.h @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -301,18 +302,25 @@ public: virtual calibration_s *get_calibration(uint8_t addr) = 0; virtual uint32_t get_device_id() const = 0; + + virtual uint8_t get_device_address() const = 0; }; -class BMP388 : public px4::ScheduledWorkItem +class BMP388 : public I2CSPIDriver { public: - BMP388(IBMP388 *interface); + BMP388(I2CSPIBusOption bus_option, int bus, IBMP388 *interface); virtual ~BMP388(); + static I2CSPIDriverBase *instantiate(const BusCLIArguments &cli, const BusInstanceIterator &iterator, + int runtime_instance); + static void print_usage(); + virtual int init(); - void print_info(); + void print_status(); + void RunImpl(); private: PX4Barometer _px4_baro; IBMP388 *_interface{nullptr}; @@ -331,9 +339,7 @@ private: bool _collect_phase{false}; - void Run() override; void start(); - void stop(); int measure(); int collect(); //get results and publish uint32_t get_measurement_time(uint8_t osr_t, uint8_t osr_p); @@ -350,6 +356,5 @@ private: /* interface factories */ -extern IBMP388 *bmp388_spi_interface(uint8_t busnum, uint32_t device); -extern IBMP388 *bmp388_i2c_interface(uint8_t busnum, uint32_t device); -typedef IBMP388 *(*BMP388_constructor)(uint8_t, uint32_t); +extern IBMP388 *bmp388_spi_interface(uint8_t busnum, uint32_t device, int bus_frequency, spi_mode_e spi_mode); +extern IBMP388 *bmp388_i2c_interface(uint8_t busnum, uint32_t device, int bus_frequency); diff --git a/src/drivers/barometer/bmp388/bmp388_i2c.cpp b/src/drivers/barometer/bmp388/bmp388_i2c.cpp index 9d46d717bc..eb0725fd05 100644 --- a/src/drivers/barometer/bmp388/bmp388_i2c.cpp +++ b/src/drivers/barometer/bmp388/bmp388_i2c.cpp @@ -41,12 +41,10 @@ #include "bmp388.h" -#if defined(PX4_I2C_OBDEV_BMP388) || defined(PX4_I2C_EXT_OBDEV_BMP388) - class BMP388_I2C: public device::I2C, public IBMP388 { public: - BMP388_I2C(uint8_t bus, uint32_t device); + BMP388_I2C(uint8_t bus, uint32_t device, int bus_frequency); virtual ~BMP388_I2C() = default; int init(); @@ -58,17 +56,19 @@ public: uint32_t get_device_id() const override { return device::I2C::get_device_id(); } + uint8_t get_device_address() const override { return device::I2C::get_device_address(); } + private: struct calibration_s _cal; }; -IBMP388 *bmp388_i2c_interface(uint8_t busnum, uint32_t device) +IBMP388 *bmp388_i2c_interface(uint8_t busnum, uint32_t device, int bus_frequency) { - return new BMP388_I2C(busnum, device); + return new BMP388_I2C(busnum, device, bus_frequency); } -BMP388_I2C::BMP388_I2C(uint8_t bus, uint32_t device) : - I2C("BMP388_I2C", nullptr, bus, device, 100 * 1000) +BMP388_I2C::BMP388_I2C(uint8_t bus, uint32_t device, int bus_frequency) : + I2C("BMP388_I2C", nullptr, bus, device, bus_frequency) { } @@ -108,5 +108,3 @@ calibration_s *BMP388_I2C::get_calibration(uint8_t addr) return nullptr; } } - -#endif /* PX4_I2C_OBDEV_BMP388 || PX4_I2C_EXT_OBDEV_BMP388 */ diff --git a/src/drivers/barometer/bmp388/bmp388_main.cpp b/src/drivers/barometer/bmp388/bmp388_main.cpp index 624cae897a..45d4097d5e 100644 --- a/src/drivers/barometer/bmp388/bmp388_main.cpp +++ b/src/drivers/barometer/bmp388/bmp388_main.cpp @@ -33,208 +33,88 @@ #include #include +#include #include "bmp388.h" -enum class BMP388_BUS { - ALL = 0, - I2C_INTERNAL, - I2C_INTERNAL1, - I2C_EXTERNAL, - SPI_INTERNAL, - SPI_EXTERNAL -}; - -namespace bmp388 +void +BMP388::print_usage() { - -// list of supported bus configurations -struct bmp388_bus_option { - BMP388_BUS busid; - BMP388_constructor interface_constructor; - uint8_t busnum; - uint32_t address; - BMP388 *dev; -} bus_options[] = { -#if defined(PX4_SPIDEV_EXT_BARO) && defined(PX4_SPI_BUS_EXT) - { 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, &bmp388_spi_interface, PX4_SPIDEV_BARO_BUS, PX4_SPIDEV_BARO, nullptr }, -# else - { 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, &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, &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, &bmp388_i2c_interface, PX4_I2C_BUS_EXPANSION, PX4_I2C_OBDEV_BMP388, nullptr }, -#endif -}; - -// find a bus structure for a busid -static struct bmp388_bus_option *find_bus(BMP388_BUS busid) -{ - for (bmp388_bus_option &bus_option : bus_options) { - if ((busid == BMP388_BUS::ALL || - busid == bus_option.busid) && bus_option.dev != nullptr) { - - return &bus_option; - } - } - - return nullptr; + PRINT_MODULE_USAGE_NAME("bmp388", "driver"); + PRINT_MODULE_USAGE_SUBCATEGORY("baro"); + PRINT_MODULE_USAGE_COMMAND("start"); + PRINT_MODULE_USAGE_PARAMS_I2C_SPI_DRIVER(true, true); + PRINT_MODULE_USAGE_PARAMS_I2C_ADDRESS(0x76); + PRINT_MODULE_USAGE_DEFAULT_COMMANDS(); } -static bool start_bus(bmp388_bus_option &bus) +I2CSPIDriverBase *BMP388::instantiate(const BusCLIArguments &cli, const BusInstanceIterator &iterator, + int runtime_instance) { - IBMP388 *interface = bus.interface_constructor(bus.busnum, bus.address); + IBMP388 *interface = nullptr; - if ((interface == nullptr) || (interface->init() != PX4_OK)) { - PX4_WARN("no device on bus %u", (unsigned)bus.busid); - delete interface; - return false; + if (iterator.busType() == BOARD_I2C_BUS) { + interface = bmp388_i2c_interface(iterator.bus(), cli.i2c_address, cli.bus_frequency); + + } else if (iterator.busType() == BOARD_SPI_BUS) { + interface = bmp388_spi_interface(iterator.bus(), iterator.devid(), cli.bus_frequency, cli.spi_mode); } - BMP388 *dev = new BMP388(interface); + if (interface == nullptr) { + PX4_ERR("failed creating interface for bus %i (devid 0x%x)", iterator.bus(), iterator.devid()); + return nullptr; + } + + if (interface->init() != OK) { + delete interface; + PX4_DEBUG("no device on bus %i (devid 0x%x)", iterator.bus(), iterator.devid()); + return nullptr; + } + + BMP388 *dev = new BMP388(iterator.configuredBusOption(), iterator.bus(), interface); if (dev == nullptr) { - PX4_ERR("alloc failed"); delete interface; - return false; + return nullptr; } - if (dev->init() != PX4_OK) { - PX4_ERR("driver start failed"); + if (OK != dev->init()) { delete dev; - return false; + return nullptr; } - bus.dev = dev; - - return true; + return dev; } -static int start(BMP388_BUS busid) -{ - for (bmp388_bus_option &bus_option : bus_options) { - if (bus_option.dev != nullptr) { - // this device is already started - PX4_WARN("already started"); - continue; - } - - if (busid != BMP388_BUS::ALL && bus_option.busid != busid) { - // not the one that is asked for - continue; - } - - if (start_bus(bus_option)) { - return PX4_OK; - } - } - - return PX4_ERROR; -} - -static int stop(BMP388_BUS busid) -{ - bmp388_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 PX4_ERROR; - } - - return PX4_OK; -} - -static int status(BMP388_BUS busid) -{ - bmp388_bus_option *bus = find_bus(busid); - - if (bus != nullptr && bus->dev != nullptr) { - bus->dev->print_info(); - return PX4_OK; - } - - 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" int bmp388_main(int argc, char *argv[]) { - int myoptind = 1; - int ch; - const char *myoptarg = nullptr; - BMP388_BUS busid = BMP388_BUS::ALL; + using ThisDriver = BMP388; + BusCLIArguments cli{true, true}; + cli.i2c_address = 0x76; + cli.default_i2c_frequency = 100 * 1000; + cli.default_spi_frequency = 10 * 1000 * 1000; - while ((ch = px4_getopt(argc, argv, "XIJSs", &myoptind, &myoptarg)) != EOF) { - switch (ch) { - case 'X': - busid = BMP388_BUS::I2C_EXTERNAL; - break; + const char *verb = cli.parseDefaultArguments(argc, argv); - case 'I': - busid = BMP388_BUS::I2C_INTERNAL; - break; - - case 'J': - busid = BMP388_BUS::I2C_INTERNAL1; - break; - - case 'S': - busid = BMP388_BUS::SPI_EXTERNAL; - break; - - case 's': - busid = BMP388_BUS::SPI_INTERNAL; - break; - - default: - return bmp388::usage(); - } + if (!verb) { + ThisDriver::print_usage(); + return -1; } - if (myoptind >= argc) { - return bmp388::usage(); - } - - const char *verb = argv[myoptind]; + BusInstanceIterator iterator(MODULE_NAME, cli, DRV_BARO_DEVTYPE_BMP388); if (!strcmp(verb, "start")) { - return bmp388::start(busid); - - } else if (!strcmp(verb, "stop")) { - return bmp388::stop(busid); - - } else if (!strcmp(verb, "status")) { - return bmp388::status(busid); + return ThisDriver::module_start(cli, iterator); } - return bmp388::usage(); + if (!strcmp(verb, "stop")) { + return ThisDriver::module_stop(iterator); + } + + if (!strcmp(verb, "status")) { + return ThisDriver::module_status(iterator); + } + + ThisDriver::print_usage(); + return -1; } diff --git a/src/drivers/barometer/bmp388/bmp388_spi.cpp b/src/drivers/barometer/bmp388/bmp388_spi.cpp index 0209112aac..94e0e25db5 100644 --- a/src/drivers/barometer/bmp388/bmp388_spi.cpp +++ b/src/drivers/barometer/bmp388/bmp388_spi.cpp @@ -46,8 +46,6 @@ #define DIR_READ (1<<7) //for set #define DIR_WRITE ~(1<<7) //for clear -#if defined(PX4_SPIDEV_BARO) || defined(PX4_SPIDEV_EXT_BARO) - #pragma pack(push,1) struct spi_data_s { uint8_t addr; @@ -63,7 +61,7 @@ struct spi_calibration_s { class BMP388_SPI: public device::SPI, public IBMP388 { public: - BMP388_SPI(uint8_t bus, uint32_t device); + BMP388_SPI(uint8_t bus, uint32_t device, int bus_frequency, spi_mode_e spi_mode); virtual ~BMP388_SPI() = default; int init(); @@ -75,17 +73,18 @@ public: uint32_t get_device_id() const override { return device::SPI::get_device_id(); } + uint8_t get_device_address() const override { return device::SPI::get_device_address(); } private: spi_calibration_s _cal; }; -IBMP388 *bmp388_spi_interface(uint8_t busnum, uint32_t device) +IBMP388 *bmp388_spi_interface(uint8_t busnum, uint32_t device, int bus_frequency, spi_mode_e spi_mode) { - return new BMP388_SPI(busnum, device); + return new BMP388_SPI(busnum, device, bus_frequency, spi_mode); } -BMP388_SPI::BMP388_SPI(uint8_t bus, uint32_t device) : - SPI("BMP388_SPI", nullptr, bus, device, SPIDEV_MODE3, 10 * 1000 * 1000) +BMP388_SPI::BMP388_SPI(uint8_t bus, uint32_t device, int bus_frequency, spi_mode_e spi_mode) : + SPI("BMP388_SPI", nullptr, bus, device, spi_mode, bus_frequency) { } @@ -125,5 +124,3 @@ calibration_s *BMP388_SPI::get_calibration(uint8_t addr) return nullptr; } } - -#endif /* PX4_SPIDEV_BARO || PX4_SPIDEV_EXT_BARO */