From dbf2d25e2f6d97be8329f171ba1f6ee756442064 Mon Sep 17 00:00:00 2001 From: Daniel Agar <daniel@agar.ca> Date: Sat, 18 Aug 2018 15:03:57 -0400 Subject: [PATCH] uORB move to standalone CDev - uORB topics should not be Devices with bus, address, device id --- src/lib/cdev/CDev.hpp | 8 ++--- src/modules/uORB/uORBCommon.hpp | 3 +- src/modules/uORB/uORBDeviceMaster.cpp | 10 ++---- src/modules/uORB/uORBDeviceMaster.hpp | 5 +-- src/modules/uORB/uORBDeviceNode.cpp | 23 ++++++------ src/modules/uORB/uORBDeviceNode.hpp | 19 +++++----- src/modules/uORB/uORBManager.cpp | 12 +++---- .../uORB/uORB_tests/uORBTest_UnitTest.cpp | 1 + .../tests/vcdev_test/vcdevtest_example.cpp | 36 +++++++------------ 9 files changed, 49 insertions(+), 68 deletions(-) diff --git a/src/lib/cdev/CDev.hpp b/src/lib/cdev/CDev.hpp index 4b6e7d314b..cecaa0f3cb 100644 --- a/src/lib/cdev/CDev.hpp +++ b/src/lib/cdev/CDev.hpp @@ -1,6 +1,6 @@ /**************************************************************************** * - * Copyright (c) 2012-2017 PX4 Development Team. All rights reserved. + * Copyright (c) 2012-2018 PX4 Development Team. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -272,7 +272,7 @@ private: bool _registered{false}; /**< true if device name was registered */ uint8_t _max_pollwaiters{0}; /**< size of the _pollset array */ - uint8_t _open_count{0}; /**< number of successful opens */ + uint16_t _open_count{0}; /**< number of successful opens */ /** * Store a pollwaiter in a slot where we can find it later. @@ -281,14 +281,14 @@ private: * * @return OK, or -errno on error. */ - int store_poll_waiter(px4_pollfd_struct_t *fds); + inline int store_poll_waiter(px4_pollfd_struct_t *fds); /** * Remove a poll waiter. * * @return OK, or -errno on error. */ - int remove_poll_waiter(px4_pollfd_struct_t *fds); + inline int remove_poll_waiter(px4_pollfd_struct_t *fds); /* do not allow copying this class */ CDev(const CDev &); diff --git a/src/modules/uORB/uORBCommon.hpp b/src/modules/uORB/uORBCommon.hpp index c9cb957269..9bb2fe015b 100644 --- a/src/modules/uORB/uORBCommon.hpp +++ b/src/modules/uORB/uORBCommon.hpp @@ -34,7 +34,6 @@ #ifndef _uORBCommon_hpp_ #define _uORBCommon_hpp_ -#include <drivers/device/device.h> #include <drivers/drv_orb_dev.h> #include <systemlib/err.h> #include "uORB.h" @@ -43,7 +42,7 @@ namespace uORB { -static const unsigned orb_maxpath = 64; +static constexpr unsigned orb_maxpath = 64; struct orb_advertdata { const struct orb_metadata *meta; diff --git a/src/modules/uORB/uORBDeviceMaster.cpp b/src/modules/uORB/uORBDeviceMaster.cpp index f3c1d686b0..c3a821af4f 100644 --- a/src/modules/uORB/uORBDeviceMaster.cpp +++ b/src/modules/uORB/uORBDeviceMaster.cpp @@ -61,16 +61,14 @@ UNUSED(node_name_str); #endif -using namespace device; - uORB::DeviceMaster::DeviceMaster() : - CDev("obj_master", TOPIC_MASTER_DEVICE_PATH) + CDev(TOPIC_MASTER_DEVICE_PATH) { _last_statistics_output = hrt_absolute_time(); } int -uORB::DeviceMaster::ioctl(device::file_t *filp, int cmd, unsigned long arg) +uORB::DeviceMaster::ioctl(cdev::file_t *filp, int cmd, unsigned long arg) { int ret; @@ -114,8 +112,6 @@ uORB::DeviceMaster::ioctl(device::file_t *filp, int cmd, unsigned long arg) *(adv->instance) = group_tries; } - const char *objname = meta->o_name; //no need for a copy, meta->o_name will never be freed or changed - /* driver wants a permanent copy of the path, so make one here */ const char *devpath = strdup(nodepath); @@ -124,7 +120,7 @@ uORB::DeviceMaster::ioctl(device::file_t *filp, int cmd, unsigned long arg) } /* construct the new node */ - uORB::DeviceNode *node = new uORB::DeviceNode(meta, objname, devpath, adv->priority); + uORB::DeviceNode *node = new uORB::DeviceNode(meta, devpath, adv->priority); /* if we didn't get a device, that's bad */ if (node == nullptr) { diff --git a/src/modules/uORB/uORBDeviceMaster.hpp b/src/modules/uORB/uORBDeviceMaster.hpp index bb73973702..0410a68ca7 100644 --- a/src/modules/uORB/uORBDeviceMaster.hpp +++ b/src/modules/uORB/uORBDeviceMaster.hpp @@ -35,6 +35,7 @@ #include <stdint.h> #include "uORBCommon.hpp" +#include <lib/cdev/CDev.hpp> namespace uORB { @@ -61,10 +62,10 @@ class Manager; * Used primarily to create new objects via the ORBIOCCREATE * ioctl. */ -class uORB::DeviceMaster : public device::CDev +class uORB::DeviceMaster : public cdev::CDev { public: - virtual int ioctl(device::file_t *filp, int cmd, unsigned long arg); + virtual int ioctl(cdev::file_t *filp, int cmd, unsigned long arg); /** * Public interface for getDeviceNodeLocked(). Takes care of synchronization. diff --git a/src/modules/uORB/uORBDeviceNode.cpp b/src/modules/uORB/uORBDeviceNode.cpp index df93ac3da4..3057d65f33 100644 --- a/src/modules/uORB/uORBDeviceNode.cpp +++ b/src/modules/uORB/uORBDeviceNode.cpp @@ -49,9 +49,7 @@ #include "uORBCommunicator.hpp" #endif /* ORB_COMMUNICATOR */ -using namespace device; - -uORB::DeviceNode::SubscriberData *uORB::DeviceNode::filp_to_sd(device::file_t *filp) +uORB::DeviceNode::SubscriberData *uORB::DeviceNode::filp_to_sd(cdev::file_t *filp) { #ifndef __PX4_NUTTX @@ -63,9 +61,8 @@ uORB::DeviceNode::SubscriberData *uORB::DeviceNode::filp_to_sd(device::file_t *f return (SubscriberData *)(FILE_PRIV(filp)); } -uORB::DeviceNode::DeviceNode(const struct orb_metadata *meta, const char *name, const char *path, - int priority, unsigned int queue_size) : - CDev(name, path), +uORB::DeviceNode::DeviceNode(const struct orb_metadata *meta, const char *path, int priority, unsigned int queue_size) : + CDev(path), _meta(meta), _priority((uint8_t)priority), _queue_size(queue_size) @@ -80,7 +77,7 @@ uORB::DeviceNode::~DeviceNode() } int -uORB::DeviceNode::open(device::file_t *filp) +uORB::DeviceNode::open(cdev::file_t *filp) { int ret; @@ -152,7 +149,7 @@ uORB::DeviceNode::open(device::file_t *filp) } int -uORB::DeviceNode::close(device::file_t *filp) +uORB::DeviceNode::close(cdev::file_t *filp) { /* is this the publisher closing? */ if (px4_getpid() == _publisher) { @@ -177,7 +174,7 @@ uORB::DeviceNode::close(device::file_t *filp) } ssize_t -uORB::DeviceNode::read(device::file_t *filp, char *buffer, size_t buflen) +uORB::DeviceNode::read(cdev::file_t *filp, char *buffer, size_t buflen) { SubscriberData *sd = (SubscriberData *)filp_to_sd(filp); @@ -233,7 +230,7 @@ uORB::DeviceNode::read(device::file_t *filp, char *buffer, size_t buflen) } ssize_t -uORB::DeviceNode::write(device::file_t *filp, const char *buffer, size_t buflen) +uORB::DeviceNode::write(cdev::file_t *filp, const char *buffer, size_t buflen) { /* * Writes are legal from interrupt context as long as the @@ -301,7 +298,7 @@ uORB::DeviceNode::write(device::file_t *filp, const char *buffer, size_t buflen) } int -uORB::DeviceNode::ioctl(device::file_t *filp, int cmd, unsigned long arg) +uORB::DeviceNode::ioctl(cdev::file_t *filp, int cmd, unsigned long arg) { SubscriberData *sd = filp_to_sd(filp); @@ -493,7 +490,7 @@ int16_t uORB::DeviceNode::topic_unadvertised(const orb_metadata *meta, int prior #endif /* ORB_COMMUNICATOR */ pollevent_t -uORB::DeviceNode::poll_state(device::file_t *filp) +uORB::DeviceNode::poll_state(cdev::file_t *filp) { SubscriberData *sd = filp_to_sd(filp); @@ -510,7 +507,7 @@ uORB::DeviceNode::poll_state(device::file_t *filp) void uORB::DeviceNode::poll_notify_one(px4_pollfd_struct_t *fds, pollevent_t events) { - SubscriberData *sd = filp_to_sd((device::file_t *)fds->priv); + SubscriberData *sd = filp_to_sd((cdev::file_t *)fds->priv); /* * If the topic looks updated to the subscriber, go ahead and notify them. diff --git a/src/modules/uORB/uORBDeviceNode.hpp b/src/modules/uORB/uORBDeviceNode.hpp index e4c6d97368..9a84943aa4 100644 --- a/src/modules/uORB/uORBDeviceNode.hpp +++ b/src/modules/uORB/uORBDeviceNode.hpp @@ -46,23 +46,22 @@ class Manager; /** * Per-object device instance. */ -class uORB::DeviceNode : public device::CDev +class uORB::DeviceNode : public cdev::CDev { public: - DeviceNode(const struct orb_metadata *meta, const char *name, const char *path, - int priority, unsigned int queue_size = 1); + DeviceNode(const struct orb_metadata *meta, const char *path, int priority, unsigned int queue_size = 1); ~DeviceNode(); /** * Method to create a subscriber instance and return the struct * pointing to the subscriber as a file pointer. */ - virtual int open(device::file_t *filp); + virtual int open(cdev::file_t *filp); /** * Method to close a subscriber for this topic. */ - virtual int close(device::file_t *filp); + virtual int close(cdev::file_t *filp); /** * reads data from a subscriber node to the buffer provided. @@ -75,7 +74,7 @@ public: * @return * ssize_t the number of bytes read. */ - virtual ssize_t read(device::file_t *filp, char *buffer, size_t buflen); + virtual ssize_t read(cdev::file_t *filp, char *buffer, size_t buflen); /** * writes the published data to the internal buffer to be read by @@ -89,12 +88,12 @@ public: * @return ssize_t * The number of bytes that are written */ - virtual ssize_t write(device::file_t *filp, const char *buffer, size_t buflen); + virtual ssize_t write(cdev::file_t *filp, const char *buffer, size_t buflen); /** * IOCTL control for the subscriber. */ - virtual int ioctl(device::file_t *filp, int cmd, unsigned long arg); + virtual int ioctl(cdev::file_t *filp, int cmd, unsigned long arg); /** * Method to publish a data to this node. @@ -177,7 +176,7 @@ public: void set_priority(uint8_t priority) { _priority = priority; } protected: - virtual pollevent_t poll_state(device::file_t *filp); + virtual pollevent_t poll_state(cdev::file_t *filp); virtual void poll_notify_one(px4_pollfd_struct_t *fds, pollevent_t events); private: @@ -211,7 +210,7 @@ private: uint8_t _queue_size; /**< maximum number of elements in the queue */ int8_t _subscriber_count{0}; - inline static SubscriberData *filp_to_sd(device::file_t *filp); + inline static SubscriberData *filp_to_sd(cdev::file_t *filp); #ifdef __PX4_NUTTX pid_t _publisher {0}; /**< if nonzero, current publisher. Only used inside the advertise call. diff --git a/src/modules/uORB/uORBManager.cpp b/src/modules/uORB/uORBManager.cpp index 3a4e0b676e..fbba90ea45 100644 --- a/src/modules/uORB/uORBManager.cpp +++ b/src/modules/uORB/uORBManager.cpp @@ -187,11 +187,8 @@ orb_advert_t uORB::Manager::orb_advertise_multi(const struct orb_metadata *meta, #endif /* ORB_USE_PUBLISHER_RULES */ - int result, fd; - orb_advert_t advertiser; - /* open the node as an advertiser */ - fd = node_open(meta, data, true, instance, priority); + int fd = node_open(meta, data, true, instance, priority); if (fd == PX4_ERROR) { PX4_ERR("%s advertise failed", meta->o_name); @@ -201,13 +198,15 @@ orb_advert_t uORB::Manager::orb_advertise_multi(const struct orb_metadata *meta, /* Set the queue size. This must be done before the first publication; thus it fails if * this is not the first advertiser. */ - result = px4_ioctl(fd, ORBIOCSETQUEUESIZE, (unsigned long)queue_size); + int result = px4_ioctl(fd, ORBIOCSETQUEUESIZE, (unsigned long)queue_size); if (result < 0 && queue_size > 1) { PX4_WARN("orb_advertise_multi: failed to set queue size"); } /* get the advertiser handle and close the node */ + orb_advert_t advertiser; + result = px4_ioctl(fd, ORBIOCGADVERTISER, (unsigned long)&advertiser); px4_close(fd); @@ -357,7 +356,8 @@ int uORB::Manager::node_open(const struct orb_metadata *meta, const void *data, int priority) { char path[orb_maxpath]; - int fd = -1, ret; + int fd = -1; + int ret = -1; /* * If meta is null, the object was not defined, i.e. it is not diff --git a/src/modules/uORB/uORB_tests/uORBTest_UnitTest.cpp b/src/modules/uORB/uORB_tests/uORBTest_UnitTest.cpp index 913f776023..8d99a95746 100644 --- a/src/modules/uORB/uORB_tests/uORBTest_UnitTest.cpp +++ b/src/modules/uORB/uORB_tests/uORBTest_UnitTest.cpp @@ -38,6 +38,7 @@ #include <stdio.h> #include <errno.h> #include <poll.h> +#include <lib/cdev/CDev.hpp> ORB_DEFINE(orb_test, struct orb_test, sizeof(orb_test), "ORB_TEST:int val;hrt_abstime time;"); ORB_DEFINE(orb_multitest, struct orb_test, sizeof(orb_test), "ORB_MULTITEST:int val;hrt_abstime time;"); diff --git a/src/platforms/posix/tests/vcdev_test/vcdevtest_example.cpp b/src/platforms/posix/tests/vcdev_test/vcdevtest_example.cpp index cafa7896e3..f9ca571944 100644 --- a/src/platforms/posix/tests/vcdev_test/vcdevtest_example.cpp +++ b/src/platforms/posix/tests/vcdev_test/vcdevtest_example.cpp @@ -43,14 +43,12 @@ #include <px4_time.h> #include "vcdevtest_example.h" #include <drivers/drv_device.h> -#include <drivers/device/device.h> +#include <lib/cdev/CDev.hpp> #include <unistd.h> #include <stdio.h> px4::AppState VCDevExample::appState; -using namespace device; - #define TESTDEV "/dev/vdevtest" static bool g_exit = false; @@ -99,27 +97,27 @@ public: size_t _read_offset; }; -class VCDevNode : public CDev +class VCDevNode : public cdev::CDev { public: VCDevNode() : - CDev("vcdevtest", TESTDEV), + CDev(TESTDEV), _is_open_for_write(false), _write_offset(0) {} ~VCDevNode() = default; - virtual int open(device::file_t *handlep); - virtual int close(device::file_t *handlep); - virtual ssize_t write(device::file_t *handlep, const char *buffer, size_t buflen); - virtual ssize_t read(device::file_t *handlep, char *buffer, size_t buflen); + virtual int open(cdev::file_t *handlep); + virtual int close(cdev::file_t *handlep); + virtual ssize_t write(cdev::file_t *handlep, const char *buffer, size_t buflen); + virtual ssize_t read(cdev::file_t *handlep, char *buffer, size_t buflen); private: bool _is_open_for_write; size_t _write_offset; char _buf[1000]; }; -int VCDevNode::open(device::file_t *handlep) +int VCDevNode::open(cdev::file_t *handlep) { // Only allow one writer if (_is_open_for_write && (handlep->flags & PX4_F_WRONLY)) { @@ -142,7 +140,7 @@ int VCDevNode::open(device::file_t *handlep) return 0; } -int VCDevNode::close(device::file_t *handlep) +int VCDevNode::close(cdev::file_t *handlep) { delete (PrivData *)handlep->priv; handlep->priv = nullptr; @@ -156,7 +154,7 @@ int VCDevNode::close(device::file_t *handlep) return 0; } -ssize_t VCDevNode::write(device::file_t *handlep, const char *buffer, size_t buflen) +ssize_t VCDevNode::write(cdev::file_t *handlep, const char *buffer, size_t buflen) { for (size_t i = 0; i < buflen && _write_offset < 1000; i++) { _buf[_write_offset] = buffer[i]; @@ -169,7 +167,7 @@ ssize_t VCDevNode::write(device::file_t *handlep, const char *buffer, size_t buf return buflen; } -ssize_t VCDevNode::read(device::file_t *handlep, char *buffer, size_t buflen) +ssize_t VCDevNode::read(cdev::file_t *handlep, char *buffer, size_t buflen) { PrivData *p = (PrivData *)handlep->priv; ssize_t chars_read = 0; @@ -289,17 +287,7 @@ int VCDevExample::main() return -px4_errno; } - void *p = nullptr; - int ret = px4_ioctl(fd, DIOC_GETPRIV, (unsigned long)&p); - - if (ret < 0) { - PX4_INFO("ioctl DIOC_GETPRIV failed %d %d", ret, px4_errno); - return -px4_errno; - } - - PX4_INFO("priv data = %p %s", p, p == (void *)_node ? "PASS" : "FAIL"); - - ret = test_pub_block(fd, 1); + int ret = test_pub_block(fd, 1); if (ret < 0) { return ret; -- GitLab