From fdc10d212be62a255e805c787016338dcceb23c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beat=20K=C3=BCng?= Date: Sat, 16 Apr 2016 10:21:52 +0200 Subject: [PATCH] orb: fix when orb_subscribe_multi is called before orb_advertise_multi This fixes the previously introduced unit test. It fixes the case where orb_subscribe_multi is called multiple times with different instances, and no publisher advertised the topic yet. In this case all subscribers got the same instance 0. --- src/modules/uORB/uORBDevices_nuttx.cpp | 17 ++++++++++++----- src/modules/uORB/uORBDevices_posix.cpp | 17 ++++++++++++----- src/modules/uORB/uORBManager.cpp | 3 +++ 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/modules/uORB/uORBDevices_nuttx.cpp b/src/modules/uORB/uORBDevices_nuttx.cpp index 55f9d353f3..dff43f7d12 100644 --- a/src/modules/uORB/uORBDevices_nuttx.cpp +++ b/src/modules/uORB/uORBDevices_nuttx.cpp @@ -571,11 +571,6 @@ uORB::DeviceMaster::ioctl(struct file *filp, int cmd, unsigned long arg) char nodepath[orb_maxpath]; uORB::DeviceNode *node; - /* set instance to zero - we could allow selective multi-pubs later based on value */ - if (adv->instance != nullptr) { - *(adv->instance) = 0; - } - /* construct a path to the node - this also checks the node name */ ret = uORB::Utils::node_mkpath(nodepath, _flavor, meta, adv->instance); @@ -592,6 +587,18 @@ uORB::DeviceMaster::ioctl(struct file *filp, int cmd, unsigned long arg) const unsigned max_group_tries = (adv->instance != nullptr) ? ORB_MULTI_MAX_INSTANCES : 1; unsigned group_tries = 0; + if (adv->instance) { + /* for an advertiser, this will be 0, but a for subscriber that requests a certain instance, + * we do not want to start with 0, but with the instance the subscriber actually requests. + */ + group_tries = *adv->instance; + + if (group_tries >= max_group_tries) { + unlock(); + return -ENOMEM; + } + } + do { /* if path is modifyable change try index */ if (adv->instance != nullptr) { diff --git a/src/modules/uORB/uORBDevices_posix.cpp b/src/modules/uORB/uORBDevices_posix.cpp index 279da29060..0d19d10cf5 100644 --- a/src/modules/uORB/uORBDevices_posix.cpp +++ b/src/modules/uORB/uORBDevices_posix.cpp @@ -577,11 +577,6 @@ uORB::DeviceMaster::ioctl(device::file_t *filp, int cmd, unsigned long arg) char nodepath[orb_maxpath]; uORB::DeviceNode *node; - /* set instance to zero - we could allow selective multi-pubs later based on value */ - if (adv->instance != nullptr) { - *(adv->instance) = 0; - } - /* construct a path to the node - this also checks the node name */ ret = uORB::Utils::node_mkpath(nodepath, _flavor, meta, adv->instance); @@ -598,6 +593,18 @@ uORB::DeviceMaster::ioctl(device::file_t *filp, int cmd, unsigned long arg) const unsigned max_group_tries = (adv->instance != nullptr) ? ORB_MULTI_MAX_INSTANCES : 1; unsigned group_tries = 0; + if (adv->instance) { + /* for an advertiser, this will be 0, but a for subscriber that requests a certain instance, + * we do not want to start with 0, but with the instance the subscriber actually requests. + */ + group_tries = *adv->instance; + + if (group_tries >= max_group_tries) { + unlock(); + return -ENOMEM; + } + } + do { /* if path is modifyable change try index */ if (adv->instance != nullptr) { diff --git a/src/modules/uORB/uORBManager.cpp b/src/modules/uORB/uORBManager.cpp index 5978965aa4..fab0e1bc74 100644 --- a/src/modules/uORB/uORBManager.cpp +++ b/src/modules/uORB/uORBManager.cpp @@ -273,6 +273,9 @@ int uORB::Manager::node_open /* open the path as either the advertiser or the subscriber */ fd = px4_open(path, advertiser ? PX4_F_WRONLY : PX4_F_RDONLY); + + } else { + *instance = 0; } /* we may need to advertise the node... */