From 670d844f08c78f5b781ab32fe768d8b54206a069 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 21 Mar 2014 10:12:33 +0400 Subject: [PATCH] ICanDriver::select() call accepts CanSelectMasks instead of raw integer masks --- libuavcan/include/uavcan/driver/can.hpp | 21 +++++++++-- libuavcan/src/transport/can_io.cpp | 22 ++++++----- libuavcan/test/node/test_node.hpp | 8 ++-- libuavcan/test/transport/can/can.hpp | 14 +++---- libuavcan/test/transport/can/iface_mock.cpp | 42 +++++++++++---------- 5 files changed, 62 insertions(+), 45 deletions(-) diff --git a/libuavcan/include/uavcan/driver/can.hpp b/libuavcan/include/uavcan/driver/can.hpp index e9d4dc9bfb..d29d10d436 100644 --- a/libuavcan/include/uavcan/driver/can.hpp +++ b/libuavcan/include/uavcan/driver/can.hpp @@ -78,6 +78,20 @@ struct CanFilterConfig uint32_t mask; }; +/** + * Events to look for during @ref ICanDriver::select() call + */ +struct CanSelectMasks +{ + uint8_t read; + uint8_t write; + + CanSelectMasks() + : read(0) + , write(0) + { } +}; + /** * Single non-blocking CAN interface. */ @@ -146,12 +160,11 @@ public: * Block until the deadline, or one of the specified interfaces becomes available for read or write. * Iface masks will be modified by the driver to indicate which exactly interfaces are available for IO. * Bit position in the masks defines interface index. - * @param [in,out] inout_write_iface_mask Mask indicating which interfaces are needed/available to write. - * @param [in,out] inout_read_iface_mask Same as above for reading. - * @param [in] blocking_deadline Zero means non-blocking operation. + * @param [in,out] inout_masks Masks indicating which interfaces are needed/available for IO. + * @param [in] blocking_deadline Zero means non-blocking operation. * @return Positive number of ready interfaces or negative error code. */ - virtual int select(int& inout_write_iface_mask, int& inout_read_iface_mask, MonotonicTime blocking_deadline) = 0; + virtual int select(CanSelectMasks& inout_masks, MonotonicTime blocking_deadline) = 0; }; } diff --git a/libuavcan/src/transport/can_io.cpp b/libuavcan/src/transport/can_io.cpp index d4c158a776..c26bdeaeed 100644 --- a/libuavcan/src/transport/can_io.cpp +++ b/libuavcan/src/transport/can_io.cpp @@ -271,18 +271,19 @@ int CanIOManager::send(const CanFrame& frame, MonotonicTime tx_deadline, Monoton { if (iface_mask == 0) break; - int write_mask = iface_mask | makePendingTxMask(); + CanSelectMasks masks; + masks.write = iface_mask | makePendingTxMask(); { - int read_mask = 0; - const int select_res = driver_.select(write_mask, read_mask, blocking_deadline); + const int select_res = driver_.select(masks, blocking_deadline); if (select_res < 0) return select_res; + assert(masks.read == 0); } // Transmission for (int i = 0; i < num_ifaces; i++) { - if (write_mask & (1 << i)) + if (masks.write & (1 << i)) { int res = 0; if (iface_mask & (1 << i)) @@ -309,7 +310,7 @@ int CanIOManager::send(const CanFrame& frame, MonotonicTime tx_deadline, Monoton // Timeout. Enqueue the frame if wasn't transmitted and leave. const bool timed_out = sysclock_.getMonotonic() >= blocking_deadline; - if (write_mask == 0 || timed_out) + if (masks.write == 0 || timed_out) { if (!timed_out) { @@ -333,10 +334,11 @@ int CanIOManager::receive(CanRxFrame& frame, MonotonicTime blocking_deadline) while (true) { - int write_mask = makePendingTxMask(); - int read_mask = (1 << num_ifaces) - 1; + CanSelectMasks masks; + masks.write = makePendingTxMask(); + masks.read = (1 << num_ifaces) - 1; { - const int select_res = driver_.select(write_mask, read_mask, blocking_deadline); + const int select_res = driver_.select(masks, blocking_deadline); if (select_res < 0) return select_res; } @@ -344,14 +346,14 @@ int CanIOManager::receive(CanRxFrame& frame, MonotonicTime blocking_deadline) // Write - if buffers are not empty, one frame will be sent for each iface per one receive() call for (int i = 0; i < num_ifaces; i++) { - if (write_mask & (1 << i)) + if (masks.write & (1 << i)) sendFromTxQueue(i); } // Read for (int i = 0; i < num_ifaces; i++) { - if (read_mask & (1 << i)) + if (masks.read & (1 << i)) { ICanIface* const iface = driver_.getIface(i); if (iface == NULL) diff --git a/libuavcan/test/node/test_node.hpp b/libuavcan/test/node/test_node.hpp index 0c1b26631f..b258fde138 100644 --- a/libuavcan/test/node/test_node.hpp +++ b/libuavcan/test/node/test_node.hpp @@ -63,13 +63,13 @@ struct PairableCanDriver : public uavcan::ICanDriver, public uavcan::ICanIface int getNumIfaces() const { return 1; } - int select(int& inout_write_iface_mask, int& inout_read_iface_mask, uavcan::MonotonicTime blocking_deadline) + int select(uavcan::CanSelectMasks& inout_masks, uavcan::MonotonicTime blocking_deadline) { assert(other); - if (inout_read_iface_mask == 1) - inout_read_iface_mask = read_queue.size() ? 1 : 0; + if (inout_masks.read == 1) + inout_masks.read = read_queue.size() ? 1 : 0; - if (inout_read_iface_mask || inout_write_iface_mask) + if (inout_masks.read || inout_masks.write) return 1; while (clock.getMonotonic() < blocking_deadline) diff --git a/libuavcan/test/transport/can/can.hpp b/libuavcan/test/transport/can/can.hpp index 348090c640..f91149866b 100644 --- a/libuavcan/test/transport/can/can.hpp +++ b/libuavcan/test/transport/can/can.hpp @@ -140,7 +140,7 @@ public: , select_failure(false) { } - int select(int& inout_write_iface_mask, int& inout_read_iface_mask, uavcan::MonotonicTime deadline) + int select(uavcan::CanSelectMasks& inout_masks, uavcan::MonotonicTime deadline) { assert(this); //std::cout << "Write/read masks: " << inout_write_iface_mask << "/" << inout_read_iface_mask << std::endl; @@ -149,21 +149,21 @@ public: return -1; const int valid_iface_mask = (1 << getNumIfaces()) - 1; - EXPECT_FALSE(inout_write_iface_mask & ~valid_iface_mask); - EXPECT_FALSE(inout_read_iface_mask & ~valid_iface_mask); + EXPECT_FALSE(inout_masks.write & ~valid_iface_mask); + EXPECT_FALSE(inout_masks.read & ~valid_iface_mask); int out_write_mask = 0; int out_read_mask = 0; for (int i = 0; i < getNumIfaces(); i++) { const int mask = 1 << i; - if ((inout_write_iface_mask & mask) && ifaces.at(i).writeable) + if ((inout_masks.write & mask) && ifaces.at(i).writeable) out_write_mask |= mask; - if ((inout_read_iface_mask & mask) && ifaces.at(i).rx.size()) + if ((inout_masks.read & mask) && ifaces.at(i).rx.size()) out_read_mask |= mask; } - inout_write_iface_mask = out_write_mask; - inout_read_iface_mask = out_read_mask; + inout_masks.write = out_write_mask; + inout_masks.read = out_read_mask; if ((out_write_mask | out_read_mask) == 0) { const uavcan::MonotonicTime ts = iclock.getMonotonic(); diff --git a/libuavcan/test/transport/can/iface_mock.cpp b/libuavcan/test/transport/can/iface_mock.cpp index ec8ad07e39..d757affc83 100644 --- a/libuavcan/test/transport/can/iface_mock.cpp +++ b/libuavcan/test/transport/can/iface_mock.cpp @@ -8,6 +8,7 @@ TEST(CanIOManager, CanDriverMock) { using uavcan::CanFrame; + using uavcan::CanSelectMasks; SystemClockMock clockmock; CanDriverMock driver(3, clockmock); @@ -15,32 +16,33 @@ TEST(CanIOManager, CanDriverMock) ASSERT_EQ(3, driver.getNumIfaces()); // All WR, no RD - int mask_wr = 7; - int mask_rd = 7; - EXPECT_LT(0, driver.select(mask_wr, mask_rd, uavcan::MonotonicTime::fromUSec(100))); - EXPECT_EQ(7, mask_wr); - EXPECT_EQ(0, mask_rd); + CanSelectMasks masks; + masks.write = 7; + masks.read = 7; + EXPECT_LT(0, driver.select(masks, uavcan::MonotonicTime::fromUSec(100))); + EXPECT_EQ(7, masks.write); + EXPECT_EQ(0, masks.read); for (int i = 0; i < 3; i++) driver.ifaces.at(i).writeable = false; // No WR, no RD - mask_wr = 7; - mask_rd = 7; - EXPECT_EQ(0, driver.select(mask_wr, mask_rd, uavcan::MonotonicTime::fromUSec(100))); - EXPECT_EQ(0, mask_wr); - EXPECT_EQ(0, mask_rd); + masks.write = 7; + masks.read = 7; + EXPECT_EQ(0, driver.select(masks, uavcan::MonotonicTime::fromUSec(100))); + EXPECT_EQ(0, masks.write); + EXPECT_EQ(0, masks.read); EXPECT_EQ(100, clockmock.monotonic); EXPECT_EQ(100, clockmock.utc); // No WR, #1 RD const CanFrame fr1 = makeCanFrame(123, "foo", EXT); driver.ifaces.at(1).pushRx(fr1); - mask_wr = 7; - mask_rd = 6; - EXPECT_LT(0, driver.select(mask_wr, mask_rd, uavcan::MonotonicTime::fromUSec(100))); - EXPECT_EQ(0, mask_wr); - EXPECT_EQ(2, mask_rd); + masks.write = 7; + masks.read = 6; + EXPECT_LT(0, driver.select(masks, uavcan::MonotonicTime::fromUSec(100))); + EXPECT_EQ(0, masks.write); + EXPECT_EQ(2, masks.read); CanFrame fr2; uavcan::MonotonicTime ts_monotonic; uavcan::UtcTime ts_utc; @@ -52,9 +54,9 @@ TEST(CanIOManager, CanDriverMock) // #0 WR, #1 RD, Select failure driver.ifaces.at(0).writeable = true; driver.select_failure = true; - mask_wr = 1; - mask_rd = 7; - EXPECT_EQ(-1, driver.select(mask_wr, mask_rd, uavcan::MonotonicTime::fromUSec(100))); - EXPECT_EQ(1, mask_wr); // Leaving masks unchanged - the library must ignore them - EXPECT_EQ(7, mask_rd); + masks.write = 1; + masks.read = 7; + EXPECT_EQ(-1, driver.select(masks, uavcan::MonotonicTime::fromUSec(100))); + EXPECT_EQ(1, masks.write); // Leaving masks unchanged - the library must ignore them + EXPECT_EQ(7, masks.read); }