From 4613cdf763aad03585e51464d4cc60b165be51da Mon Sep 17 00:00:00 2001 From: Philippe Leduc Date: Thu, 1 Oct 2026 12:20:26 +0200 Subject: [PATCH] Bus: map a process image configured outside createMapping - Map a process image whose slave mappings were set by other means. - Refuse process data mappings the frames cannot carry. - Refuse datagrams larger than a frame. --- lib/master/include/kickcat/Bus.h | 9 ++ lib/master/src/Bus.cc | 218 ++++++++++++++++++++----------- lib/master/src/Link.cc | 4 + unit/src/bus-t.cc | 168 ++++++++++++++++++++++++ unit/src/dc-t.cc | 2 +- unit/src/link-t.cc | 7 + 6 files changed, 328 insertions(+), 80 deletions(-) diff --git a/lib/master/include/kickcat/Bus.h b/lib/master/include/kickcat/Bus.h index df8c3917..6d5b74cf 100644 --- a/lib/master/include/kickcat/Bus.h +++ b/lib/master/include/kickcat/Bus.h @@ -96,6 +96,13 @@ namespace kickcat /// \brief Like createMapping(iomap), but throws if iomap_size cannot hold the process image. void createMapping(uint8_t* iomap, std::size_t iomap_size); + /// \brief Bind the slaves process data to iomap and build the cyclic frames, without touching the slaves. + /// \details Each slave input/output must hold its logical address and byte size: createMapping() sets them, + /// or the slaves were configured by other means. The mailbox status check additionally needs the + /// FMMUs createMapping() programs. Throws if a block does not fit in a frame or if iomap_size + /// cannot hold the process image. + void mapProcessImage(uint8_t* iomap, std::size_t iomap_size); + std::vector& slaves() { return slaves_; } // asynchrone read/write/mailbox/state methods @@ -244,6 +251,8 @@ namespace kickcat // mapping helpers void detectMapping(); + void assignLogicalAddresses(); + void buildFrames(); void readMappedPDO(Slave& slave, uint16_t index); void configureFMMUs(); void configureMailboxFMMUs(); diff --git a/lib/master/src/Bus.cc b/lib/master/src/Bus.cc index b11104e7..8afdd6a5 100644 --- a/lib/master/src/Bus.cc +++ b/lib/master/src/Bus.cc @@ -569,66 +569,168 @@ namespace kickcat void Bus::createMapping(uint8_t* iomap, std::size_t iomap_size) { - // First we need to know: - // - how many bits to map per slave - // - which SM to use - // - logical offset in the frame detectMapping(); + assignLogicalAddresses(); + mapProcessImage(iomap, iomap_size); - // Second step: create 'block I/O' lists for read and write op - // Note A: offset computing will overlap input and output in the frame (better density and compatibility, more works for master) - // Note B: a frame cannot handle more than 1486 bytes - // Full reset: a previous mapping would otherwise leak its block IO lists into this one. - pi_frames_.clear(); - pi_frames_.push_back(PIFrame{}); - pi_frames_[0].description.address = 0; - std::vector> frame_mbx_slaves(1); + configureFMMUs(); + if (mailbox_status_fmmu_ != MailboxStatusFMMU::NONE) + { + configureMailboxFMMUs(); + } + } + + + void Bus::assignLogicalAddresses() + { + // Inputs and outputs of a slave overlap in the frame (better density), and a frame + // cannot hold more than 1486 bytes: a slave that would overflow starts the next one. uint32_t address = 0; + uint32_t frame_end = MAX_ETHERCAT_PAYLOAD_SIZE; for (auto& slave : slaves_) { - // get the biggest one. - int32_t size = std::max(slave.input.bsize, slave.output.bsize); - if ((address + size) > (pi_frames_.size() * MAX_ETHERCAT_PAYLOAD_SIZE)) // do we overflow current frame ? + uint32_t size = static_cast(std::max(slave.input.bsize, slave.output.bsize)); + if ((address + size) > frame_end) { - auto& desc = pi_frames_.back().description; - desc.logical_size = address - desc.address; // frame size = current address - frame address + address = frame_end; + frame_end += MAX_ETHERCAT_PAYLOAD_SIZE; + } + slave.input.address = address; + slave.output.address = address; + address += size; + } + } - // current size will overflow the frame at the current offset: set in on the next frame - address = static_cast(pi_frames_.size()) * MAX_ETHERCAT_PAYLOAD_SIZE; - PIFrame new_frame{}; - new_frame.description.address = address; - pi_frames_.push_back(std::move(new_frame)); - frame_mbx_slaves.push_back({}); + + void Bus::mapProcessImage(uint8_t* iomap, std::size_t iomap_size) + { + std::size_t required = 0; + for (auto const& slave : slaves_) + { + for (Slave::PIMapping const* mapping : {&slave.input, &slave.output}) + { + if ((mapping->bsize < 0) or (mapping->bsize > MAX_ETHERCAT_PAYLOAD_SIZE)) + { + THROW_ERROR("a process data block does not fit in a frame"); + } + if ((static_cast(mapping->address) + static_cast(mapping->bsize)) > UINT32_MAX + uint64_t{1}) + { + THROW_ERROR("a process data block exceeds the logical address space"); + } + required += static_cast(mapping->bsize); } + } + if (required > iomap_size) + { + THROW_ERROR("iomap buffer too small for the process image"); + } - // create block IO entries - PIFrame& current_frame = pi_frames_.back(); + uint8_t* pos = iomap; + for (auto& slave : slaves_) + { if (slave.input.bsize > 0) { - current_frame.inputs.push_back ({nullptr, address - current_frame.description.address, slave.input.bsize, &slave}); + slave.input.data = pos; + pos += slave.input.bsize; + } + } + for (auto& slave : slaves_) + { + if (slave.output.bsize > 0) + { + slave.output.data = pos; + pos += slave.output.bsize; } + } + + buildFrames(); + } + + + void Bus::buildFrames() + { + struct Block + { + Slave* slave; + Slave::PIMapping* mapping; + bool is_input; + }; + std::vector blocks; + for (auto& slave : slaves_) + { + if (slave.input.bsize > 0) + { + blocks.push_back({&slave, &slave.input, true}); + } if (slave.output.bsize > 0) { - current_frame.outputs.push_back({nullptr, address - current_frame.description.address, slave.output.bsize, &slave}); + blocks.push_back({&slave, &slave.output, false}); } + } + std::stable_sort(blocks.begin(), blocks.end(), [](Block const& a, Block const& b) + { + return a.mapping->address < b.mapping->address; + }); - // save mapping offset (need to configure slave FMMU) - slave.input.address = address; - slave.output.address = address; + // Full reset: a previous mapping would otherwise leak its block IO lists into this one. + pi_frames_.clear(); + pi_frames_.push_back(PIFrame{}); + if (not blocks.empty()) + { + pi_frames_[0].description.address = blocks.front().mapping->address; + } - // update offset - address += size; + std::vector frame_of_slave(slaves_.size(), -1); + for (auto const& block : blocks) + { + uint64_t end = static_cast(block.mapping->address) + static_cast(block.mapping->bsize); + if ((end - pi_frames_.back().description.address) > MAX_ETHERCAT_PAYLOAD_SIZE) + { + auto const& previous = pi_frames_.back().description; + if (block.mapping->address < previous.address + static_cast(previous.logical_size)) + { + THROW_ERROR("overlapping process data blocks cannot be split into frames"); + } + PIFrame new_frame{}; + new_frame.description.address = block.mapping->address; + pi_frames_.push_back(std::move(new_frame)); + } + + PIFrame& frame = pi_frames_.back(); + uint32_t offset = block.mapping->address - frame.description.address; + blockIO bio{block.mapping->data, offset, block.mapping->bsize, block.slave}; + if (block.is_input) + { + frame.inputs.push_back(bio); + } + else + { + frame.outputs.push_back(bio); + } + frame.description.logical_size = std::max(frame.description.logical_size, static_cast(offset) + block.mapping->bsize); - if (slave.sii.info.mailbox_protocol != 0) + std::ptrdiff_t position = block.slave - slaves_.data(); + if (frame_of_slave[position] < 0) { - frame_mbx_slaves.back().push_back(&slave); + frame_of_slave[position] = static_cast(pi_frames_.size() - 1); } } - // update last frame size - auto& last_desc = pi_frames_.back().description; - last_desc.logical_size = address - last_desc.address; + // A mailbox slave without process data joins the frame of the slave before it + std::vector> frame_mbx_slaves(pi_frames_.size()); + int32_t current_frame = 0; + for (std::size_t i = 0; i < slaves_.size(); ++i) + { + if (frame_of_slave[i] >= 0) + { + current_frame = frame_of_slave[i]; + } + if (slaves_[i].sii.info.mailbox_protocol != 0) + { + frame_mbx_slaves[current_frame].push_back(&slaves_[i]); + } + } // Set pdo_size for all frames (equals logical_size before mailbox status extension) for (auto& frame : pi_frames_) @@ -761,48 +863,6 @@ namespace kickcat descriptions.push_back(frame.description); } link_->setLogicalMapping(descriptions); - - // Validate the client buffer can hold the process image before writing into it. - std::size_t required = 0; - for (auto const& frame : pi_frames_) - { - for (auto const& bio : frame.inputs) { required += bio.size; } - for (auto const& bio : frame.outputs) { required += bio.size; } - } - if (required > iomap_size) - { - THROW_ERROR("createMapping: iomap buffer too small for the process image"); - } - - // Third step: associate client buffer address to block IO and slaves - // Note: inputs are mapped first, outputs second - uint8_t* pos = iomap; - for (auto& frame : pi_frames_) - { - for (auto& bio : frame.inputs) - { - bio.iomap = pos; - bio.slave->input.data = pos; - pos += bio.size; - } - } - for (auto& frame : pi_frames_) - { - for (auto& bio : frame.outputs) - { - bio.iomap = pos; - bio.slave->output.data = pos; - pos += bio.size; - } - } - - // Fourth step: program FMMUs and SyncManagers - configureFMMUs(); - - if (mailbox_status_fmmu_ != MailboxStatusFMMU::NONE) - { - configureMailboxFMMUs(); - } } diff --git a/lib/master/src/Link.cc b/lib/master/src/Link.cc index 78702b4b..b8b9e250 100644 --- a/lib/master/src/Link.cc +++ b/lib/master/src/Link.cc @@ -74,6 +74,10 @@ namespace kickcat { THROW_ERROR("Too many datagrams in flight. Max is 255"); } + if (data_size > MAX_ETHERCAT_PAYLOAD_SIZE) + { + THROW_ERROR("Datagram data does not fit in a frame"); + } link_info("Adding a datagram (already %d pending)\n", index_queue_); uint16_t const needed_space = datagram_size(data_size); diff --git a/unit/src/bus-t.cc b/unit/src/bus-t.cc index 1e8af683..eae474bd 100644 --- a/unit/src/bus-t.cc +++ b/unit/src/bus-t.cc @@ -45,6 +45,7 @@ struct BusAccessor : public Bus { using Bus::Bus; using Bus::pi_frames_; + using Bus::assignLogicalAddresses; }; @@ -1673,3 +1674,170 @@ TEST_F(BusTest, logical_mapping_shared_with_link_at_mapping) ASSERT_EQ(0, desc.entries[0].input_offset); ASSERT_EQ(32, desc.entries[0].input_size); } + + +TEST_F(BusTest, mapProcessImage_throws_when_iomap_too_small) +{ + auto& slave = bus.slaves().at(0); + slave.input.address = 0x10000; + slave.input.bsize = 16; + + uint8_t too_small[8]; + ASSERT_THROW(bus.mapProcessImage(too_small, sizeof(too_small)), Error); +} + + +TEST_F(BusTest2Slaves, mapProcessImage_disjoint_inputs_and_outputs_share_a_frame) +{ + auto& slave0 = bus.slaves().at(0); + auto& slave1 = bus.slaves().at(1); + slave0.sii.info.mailbox_protocol = eeprom::MailboxProtocol::None; + slave1.sii.info.mailbox_protocol = eeprom::MailboxProtocol::None; + slave0.output = {nullptr, 88, 11, 2, 0x10000}; + slave1.output = {nullptr, 88, 11, 2, 0x1000B}; + slave0.input = {nullptr, 88, 11, 3, 0x10016}; + slave1.input = {nullptr, 88, 11, 3, 0x10021}; + + uint8_t iomap[44]; + bus.mapProcessImage(iomap, sizeof(iomap)); + ASSERT_TRUE(mock_link->pendingDatagrams().empty()); + + ASSERT_EQ(iomap, slave0.input.data); + ASSERT_EQ(iomap + 11, slave1.input.data); + ASSERT_EQ(iomap + 22, slave0.output.data); + ASSERT_EQ(iomap + 33, slave1.output.data); + + ASSERT_EQ(1u, bus.pi_frames_.size()); + auto const& frame = bus.pi_frames_[0]; + ASSERT_EQ(0x10000u, frame.description.address); + ASSERT_EQ(44, frame.description.logical_size); + ASSERT_EQ(6, frame.expected_lrw_wkc); + ASSERT_EQ(2u, frame.inputs.size()); + ASSERT_EQ(22u, frame.inputs[0].offset); + ASSERT_EQ(33u, frame.inputs[1].offset); + ASSERT_EQ(2u, frame.outputs.size()); + ASSERT_EQ(0u, frame.outputs[0].offset); + ASSERT_EQ(11u, frame.outputs[1].offset); + ASSERT_EQ(1u, mock_link->logicalMapping().size()); + + for (uint8_t i = 0; i < 11; ++i) + { + slave0.output.data[i] = static_cast(0xA0 + i); + slave1.output.data[i] = static_cast(0xB0 + i); + } + bus.sendLogicalReadWrite([](DatagramState const&){}); + ASSERT_EQ(1u, mock_link->pendingDatagrams().size()); + auto const& lrw = mock_link->pendingDatagrams()[0]; + ASSERT_EQ(Command::LRW, lrw.command); + ASSERT_EQ(0x10000u, lrw.address); + ASSERT_EQ(44, lrw.data_size); + ASSERT_EQ(0xA0, lrw.data[0]); + ASSERT_EQ(0xB0, lrw.data[11]); + + std::array reply{}; + reply[22] = 0x12; + reply[33] = 0x34; + mock_link->handleProcess(Command::LRW, reply, 6); + bus.processAwaitingFrames(); + ASSERT_EQ(0x12, slave0.input.data[0]); + ASSERT_EQ(0x34, slave1.input.data[0]); +} + + +TEST_F(BusTest2Slaves, mapProcessImage_splits_distant_windows) +{ + auto& slave0 = bus.slaves().at(0); + auto& slave1 = bus.slaves().at(1); + slave0.sii.info.mailbox_protocol = eeprom::MailboxProtocol::None; + slave1.sii.info.mailbox_protocol = eeprom::MailboxProtocol::None; + slave0.input = {nullptr, 112, 14, 3, 0x11000}; + slave0.output = {nullptr, 0, 0, 0, 0}; + slave1.input = {nullptr, 0, 0, 0, 0}; + slave1.output = {nullptr, 72, 9, 2, 0x12000}; + + uint8_t iomap[23]; + bus.mapProcessImage(iomap, sizeof(iomap)); + + ASSERT_EQ(2u, bus.pi_frames_.size()); + ASSERT_EQ(0x11000u, bus.pi_frames_[0].description.address); + ASSERT_EQ(14, bus.pi_frames_[0].description.logical_size); + ASSERT_EQ(1, bus.pi_frames_[0].expected_lrw_wkc); + ASSERT_EQ(1u, bus.pi_frames_[0].inputs.size()); + ASSERT_TRUE(bus.pi_frames_[0].outputs.empty()); + + ASSERT_EQ(0x12000u, bus.pi_frames_[1].description.address); + ASSERT_EQ(9, bus.pi_frames_[1].description.logical_size); + ASSERT_EQ(2, bus.pi_frames_[1].expected_lrw_wkc); + ASSERT_TRUE(bus.pi_frames_[1].inputs.empty()); + ASSERT_EQ(0u, bus.pi_frames_[1].outputs[0].offset); +} + + +TEST_F(BusTest2Slaves, createMapping_layout_splits_frames_at_payload_boundary) +{ + // createMapping layout: slave 1 overflows the first 1486-byte window and starts the next one; + // each mailbox slave gets its status bit in the frame holding its process data. + auto& slave0 = bus.slaves().at(0); + auto& slave1 = bus.slaves().at(1); + bus.configureMailboxStatusCheck(MailboxStatusFMMU::READ_CHECK); + slave0.input = {nullptr, 8000, 1000, 3, 0}; + slave0.output = {nullptr, 6400, 800, 2, 0}; + slave1.input = {nullptr, 4800, 600, 3, 0}; + slave1.output = {nullptr, 0, 0, 0, 0}; + + bus.assignLogicalAddresses(); + std::vector iomap(2400); + bus.mapProcessImage(iomap.data(), iomap.size()); + + ASSERT_EQ(0u, slave0.input.address); + ASSERT_EQ(0u, slave0.output.address); + ASSERT_EQ(1486u, slave1.input.address); + + ASSERT_EQ(2u, bus.pi_frames_.size()); + auto const& frame0 = bus.pi_frames_[0]; + auto const& frame1 = bus.pi_frames_[1]; + ASSERT_EQ(0u, frame0.description.address); + ASSERT_EQ(1000, frame0.description.pdo_size); + ASSERT_EQ(1004, frame0.description.logical_size); // 3-byte FMMU separation + 1 status byte + ASSERT_EQ(1486u, frame1.description.address); + ASSERT_EQ(600, frame1.description.pdo_size); + ASSERT_EQ(604, frame1.description.logical_size); + + ASSERT_EQ(1u, frame0.mailbox_read_status.size()); + ASSERT_EQ(&slave0, frame0.mailbox_read_status[0].slave); + ASSERT_EQ(1003u, frame0.mailbox_read_status[0].byte_offset); + ASSERT_EQ(1u, frame1.mailbox_read_status.size()); + ASSERT_EQ(&slave1, frame1.mailbox_read_status[0].slave); + ASSERT_EQ(603u, frame1.mailbox_read_status[0].byte_offset); + + ASSERT_EQ(3, frame0.expected_lrw_wkc); + ASSERT_EQ(1, frame1.expected_lrw_wkc); + ASSERT_EQ(iomap.data(), slave0.input.data); + ASSERT_EQ(iomap.data() + 1000, slave1.input.data); + ASSERT_EQ(iomap.data() + 1600, slave0.output.data); +} + + +TEST_F(BusTest2Slaves, mapProcessImage_refuses_invalid_blocks) +{ + auto& slave0 = bus.slaves().at(0); + auto& slave1 = bus.slaves().at(1); + std::vector iomap(4096); + + slave0.input = {nullptr, 0, 16, 3, 0}; + slave0.output = {nullptr, 0, -8, 2, 0}; + ASSERT_THROW(bus.mapProcessImage(iomap.data(), 8), Error); + + slave0.output = {nullptr, 0, MAX_ETHERCAT_PAYLOAD_SIZE + 1, 2, 0x1000}; + ASSERT_THROW(bus.mapProcessImage(iomap.data(), iomap.size()), Error); + + slave0.output = {nullptr, 0, 16, 2, 0xFFFFFFF8}; + ASSERT_THROW(bus.mapProcessImage(iomap.data(), iomap.size()), Error); + + // Same-address input/output of slave 1 straddling the first 1486-byte window + slave0.input = {nullptr, 0, 1400, 3, 0}; + slave0.output = {nullptr, 0, 0, 0, 0}; + slave1.input = {nullptr, 0, 10, 3, 1400}; + slave1.output = {nullptr, 0, 100, 2, 1400}; + ASSERT_THROW(bus.mapProcessImage(iomap.data(), iomap.size()), Error); +} diff --git a/unit/src/dc-t.cc b/unit/src/dc-t.cc index be027647..516a9bae 100644 --- a/unit/src/dc-t.cc +++ b/unit/src/dc-t.cc @@ -48,7 +48,7 @@ namespace class DropFilterSocket final : public AbstractSocket { public: - explicit DropFilterSocket(std::shared_ptr inner) + DropFilterSocket(std::shared_ptr inner) : inner_(std::move(inner)) { } diff --git a/unit/src/link-t.cc b/unit/src/link-t.cc index 2c11ad4c..85257d23 100644 --- a/unit/src/link-t.cc +++ b/unit/src/link-t.cc @@ -468,6 +468,13 @@ TEST_F(LinkTest, isRedundancyNeeded_no_interfaces) ASSERT_EQ(is_redundancy_activated, false); } +TEST_F(LinkTest, addDatagram_larger_than_a_frame) +{ + std::vector data(MAX_ETHERCAT_PAYLOAD_SIZE + 1); + ASSERT_THROW(link.addDatagram(Command::LWR, 0, data.data(), static_cast(data.size()), nullptr, nullptr), Error); + ASSERT_EQ(sentFrames(), 0); +} + TEST_F(LinkTest, sendFrame_error_wrong_number_write) { link.addDatagram(Command::BRD, createAddress(0, 0x0000), nullptr, nullptr, nullptr);