From 9da537c0926d015de1f30dbdde4cdbe031f7e4ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beat=20K=C3=BCng?= Date: Wed, 27 Apr 2016 11:23:05 +0200 Subject: [PATCH] uorb: add uorb_unadvertise method This is necessary when using multiple instances of a topic. However it does not free the underlying resources, as it is assumed they will be used again at a later point. --- src/modules/uORB/Publication.cpp | 1 + src/modules/uORB/uORB.cpp | 5 +++++ src/modules/uORB/uORB.h | 5 +++++ src/modules/uORB/uORBDevices_nuttx.cpp | 24 ++++++++++++++++++++++++ src/modules/uORB/uORBDevices_nuttx.hpp | 5 ++++- src/modules/uORB/uORBDevices_posix.cpp | 24 ++++++++++++++++++++++++ src/modules/uORB/uORBDevices_posix.hpp | 5 ++++- src/modules/uORB/uORBManager.cpp | 5 +++++ src/modules/uORB/uORBManager.hpp | 8 ++++++++ 9 files changed, 80 insertions(+), 2 deletions(-) diff --git a/src/modules/uORB/Publication.cpp b/src/modules/uORB/Publication.cpp index e979e3b4f4..9990d8d543 100644 --- a/src/modules/uORB/Publication.cpp +++ b/src/modules/uORB/Publication.cpp @@ -96,6 +96,7 @@ void PublicationBase::update(void *data) PublicationBase::~PublicationBase() { + orb_unadvertise(getHandle()); } PublicationNode::PublicationNode(const struct orb_metadata *meta, diff --git a/src/modules/uORB/uORB.cpp b/src/modules/uORB/uORB.cpp index e37a5772a7..375f261ca8 100644 --- a/src/modules/uORB/uORB.cpp +++ b/src/modules/uORB/uORB.cpp @@ -51,6 +51,11 @@ orb_advert_t orb_advertise_multi(const struct orb_metadata *meta, const void *da return uORB::Manager::get_instance()->orb_advertise_multi(meta, data, instance, priority); } +int orb_unadvertise(orb_advert_t handle) +{ + return uORB::Manager::get_instance()->orb_unadvertise(handle); +} + int orb_publish_auto(const struct orb_metadata *meta, orb_advert_t *handle, const void *data, int *instance, int priority) { diff --git a/src/modules/uORB/uORB.h b/src/modules/uORB/uORB.h index 5e07fc5c3e..807da7b75b 100644 --- a/src/modules/uORB/uORB.h +++ b/src/modules/uORB/uORB.h @@ -142,6 +142,11 @@ extern orb_advert_t orb_advertise(const struct orb_metadata *meta, const void *d extern orb_advert_t orb_advertise_multi(const struct orb_metadata *meta, const void *data, int *instance, int priority) __EXPORT; +/** + * @see uORB::Manager::orb_unadvertise() + */ +extern int orb_unadvertise(orb_advert_t handle) __EXPORT; + /** * Advertise as the publisher of a topic. * diff --git a/src/modules/uORB/uORBDevices_nuttx.cpp b/src/modules/uORB/uORBDevices_nuttx.cpp index 994f16e1ac..c39612f686 100644 --- a/src/modules/uORB/uORBDevices_nuttx.cpp +++ b/src/modules/uORB/uORBDevices_nuttx.cpp @@ -336,6 +336,30 @@ uORB::DeviceNode::publish return OK; } +int uORB::DeviceNode::unadvertise(orb_advert_t handle) +{ + if (handle == nullptr) { + return -EINVAL; + } + + uORB::DeviceNode *devnode = (uORB::DeviceNode *)handle; + + /* + * We are cheating a bit here. First, with the current implementation, we can only + * have multiple publishers for instance 0. In this case the caller will have + * instance=nullptr and _published has no effect at all. Thus no unadvertise is + * necessary. + * In case of multiple instances, we have at most 1 publisher per instance and + * we can signal an instance as 'free' by setting _published to false. + * We never really free the DeviceNode, for this we would need reference counting + * of subscribers and publishers. But we also do not have a leak since future + * publishers reuse the same DeviceNode object. + */ + devnode->_published = false; + + return PX4_OK; +} + pollevent_t uORB::DeviceNode::poll_state(struct file *filp) { diff --git a/src/modules/uORB/uORBDevices_nuttx.hpp b/src/modules/uORB/uORBDevices_nuttx.hpp index 2c9ea26719..0401b0db28 100644 --- a/src/modules/uORB/uORBDevices_nuttx.hpp +++ b/src/modules/uORB/uORBDevices_nuttx.hpp @@ -122,6 +122,8 @@ public: const void *data ); + static int unadvertise(orb_advert_t handle); + /** * processes a request for add subscription from remote * @param rateInHz @@ -184,7 +186,8 @@ private: uint8_t *_data; /**< allocated object buffer */ hrt_abstime _last_update; /**< time the object was last updated */ volatile unsigned _generation; /**< object generation count */ - pid_t _publisher; /**< if nonzero, current publisher */ + pid_t _publisher; /**< if nonzero, current publisher. Only used inside the advertise call. + We allow one publisher to have an open file descriptor at the same time. */ const int _priority; /**< priority of topic */ bool _published; /**< has ever data been published */ diff --git a/src/modules/uORB/uORBDevices_posix.cpp b/src/modules/uORB/uORBDevices_posix.cpp index 6d2d8d027b..d631b90f5b 100644 --- a/src/modules/uORB/uORBDevices_posix.cpp +++ b/src/modules/uORB/uORBDevices_posix.cpp @@ -352,6 +352,30 @@ uORB::DeviceNode::publish(const orb_metadata *meta, orb_advert_t handle, const v return PX4_OK; } +int uORB::DeviceNode::unadvertise(orb_advert_t handle) +{ + if (handle == nullptr) { + return -EINVAL; + } + + uORB::DeviceNode *devnode = (uORB::DeviceNode *)handle; + + /* + * We are cheating a bit here. First, with the current implementation, we can only + * have multiple publishers for instance 0. In this case the caller will have + * instance=nullptr and _published has no effect at all. Thus no unadvertise is + * necessary. + * In case of multiple instances, we have at most 1 publisher per instance and + * we can signal an instance as 'free' by setting _published to false. + * We never really free the DeviceNode, for this we would need reference counting + * of subscribers and publishers. But we also do not have a leak since future + * publishers reuse the same DeviceNode object. + */ + devnode->_published = false; + + return PX4_OK; +} + pollevent_t uORB::DeviceNode::poll_state(device::file_t *filp) { diff --git a/src/modules/uORB/uORBDevices_posix.hpp b/src/modules/uORB/uORBDevices_posix.hpp index ac2f36fcb4..fbaf28a30a 100644 --- a/src/modules/uORB/uORBDevices_posix.hpp +++ b/src/modules/uORB/uORBDevices_posix.hpp @@ -59,6 +59,8 @@ public: static ssize_t publish(const orb_metadata *meta, orb_advert_t handle, const void *data); + static int unadvertise(orb_advert_t handle); + /** * processes a request for add subscription from remote * @param rateInHz @@ -122,7 +124,8 @@ private: uint8_t *_data; /**< allocated object buffer */ hrt_abstime _last_update; /**< time the object was last updated */ volatile unsigned _generation; /**< object generation count */ - unsigned long _publisher; /**< if nonzero, current publisher */ + unsigned long _publisher; /**< if nonzero, current publisher. Only used inside the advertise call. + We allow one publisher to have an open file descriptor at the same time. */ const int _priority; /**< priority of topic */ bool _published; /**< has ever data been published */ diff --git a/src/modules/uORB/uORBManager.cpp b/src/modules/uORB/uORBManager.cpp index fab0e1bc74..601d8597e2 100644 --- a/src/modules/uORB/uORBManager.cpp +++ b/src/modules/uORB/uORBManager.cpp @@ -129,6 +129,11 @@ orb_advert_t uORB::Manager::orb_advertise_multi(const struct orb_metadata *meta, return advertiser; } +int uORB::Manager::orb_unadvertise(orb_advert_t handle) +{ + return uORB::DeviceNode::unadvertise(handle); +} + int uORB::Manager::orb_subscribe(const struct orb_metadata *meta) { return node_open(PUBSUB, meta, nullptr, false); diff --git a/src/modules/uORB/uORBManager.hpp b/src/modules/uORB/uORBManager.hpp index 094dfa96b6..93d2f41119 100644 --- a/src/modules/uORB/uORBManager.hpp +++ b/src/modules/uORB/uORBManager.hpp @@ -130,6 +130,14 @@ public: int priority) ; + /** + * Unadvertise a topic. + * + * @param handle handle returned by orb_advertise or orb_advertise_multi. + * @return 0 on success + */ + int orb_unadvertise(orb_advert_t handle); + /** * Publish new data to a topic. *