fix single device inputs/outputs overruning segment size by ddi-kkugler · Pull Request #907 · OpenEtherCATsociety/SOEM

ddi-kkugler · 2025-04-14T17:47:12Z

certain modular devices can have process data that exceeds a segment size. Add code to allow multiple segments within a single device.

ddi-kkugler · 2025-04-16T18:56:51Z

I believe this also fixes #243

ddi-kkugler · 2025-04-21T16:44:57Z

I sent in my contributors agreement on April 11th, what else do I need to do to have this reviewed?

nakarlsson · 2025-04-23T06:09:05Z

Can you provide the config to trigger this? A slaveinfo output would do

ddi-kkugler · 2025-04-24T03:17:20Z

Output from slave device with output larger than one segment. Device has modular profile that can be quite large.

SOEM (Simple Open EtherCAT Master)
Slaveinfo
Starting slaveinfo
ec_init on enx3c8cf8600c1c succeeded.
1 slaves found and configured.
Calculated workcounter 3

Slave:1
 Name:IO Controllers
 Output size: 5856bits
 Input size: 12216bits
 State: 4
 Delay: 0[ns]
 Has DC: 1
 DCParentport:0
 Activeports:0.1.0.0
 Configured address: 1001
 Man: 0000076b ID: 00000064 Rev: 00041180
 SM0 A:1000 L: 128 F:00010026 Type:1
 SM1 A:1600 L: 128 F:00010022 Type:2
 SM2 A:1c00 L: 732 F:00010064 Type:3
 SM3 A:5600 L:1527 F:00010020 Type:4
 FMMU0 Ls:00000000 Ll: 732 Lsb:0 Leb:7 Ps:1c00 Psb:0 Ty:02 Act:01
 FMMU1 Ls:000002dc Ll:1527 Lsb:0 Leb:7 Ps:5600 Psb:0 Ty:01 Act:01
 FMMUfunc 0:1 1:2 2:3 3:0
 MBX length wr: 128 rd: 128 MBX protocols : 0c
 CoE details: 2f FoE details: 01 EoE details: 00 SoE details: 00
 Ebus current: 0[mA]
 only LRD/LWR:0
End slaveinfo, close socket
End program

ddi-kkugler · 2025-04-24T03:23:38Z

I just noticed that with this large config the actual wkc returned is 4 instead of 3. The expected wkc is still calculated as 3. Looking through spec docs now to see if there is any mention about what should happen in this case.

ddi-kkugler · 2025-04-24T04:15:19Z

I just noticed that with this large config the actual wkc returned is 4 instead of 3. The expected wkc is still calculated as 3. Looking through spec docs now to see if there is any mention about what should happen in this case.

ETG1000.4 indicates that the wkc is associated with each ECAT PDU. Since adding additional segments is adding more PDUs for the process data the WKC should be updated accordingly. For the slave config I have posted above the correct WKC should be 4. I pushed the changes accordingly.

nakarlsson · 2025-04-24T07:42:38Z

Have you checked/verified the overlapped API also?

ddi-kkugler · 2025-04-25T00:32:02Z

I don't know enough to know how to test that. If I try calling config_overlap_map then the device I have seems to not report data correctly.

EDIT: Using send_overlap_processdata with config_overlap_map the system works as expected. This is a test with only a single device on the ecat network.

nakarlsson · 2025-04-25T05:54:38Z

I don't know enough to know how to test that. If I try calling config_overlap_map then the device I have seems to not report data correctly.

EDIT: Using send_overlap_processdata with config_overlap_map the system works as expected. This is a test with only a single device on the ecat network.

Sounds good, I assume first try using config_overlap_map was done using send_processdata?

ddi-kkugler · 2025-04-25T15:11:05Z

Sounds good, I assume first try using config_overlap_map was done using send_processdata?

Yes, exactly. Once I changed both over it worked as expected.

ddi-kkugler · 2025-04-28T03:27:10Z

Let me know anything else you want to test/try before getting this merged.

soem/ethercatconfig.c

nakarlsson · 2025-04-28T12:37:24Z

soem/ethercatconfig.c

                     currentsegment++;
-                     segmentsize = diff;
                  }
+                  while (diff > (EC_MAXLRWDATA - EC_FIRSTDCDATAGRAM) && currentsegment < (EC_MAXIOSEGMENTS - 1))


I wonder if there are a clever way to split a "big" segment? I did a sample slave with 4B Out/1540B In.
The logic will create 3 frames when 2 frames easily is enough, but sometimes the clever logic comes with a cost in complexity to maintain and test.

When running overlapped the DC frame is sent alone in first segment.
Also, EC_MAXLRWDATA - EC_FIRSTDCDATAGRAM , EC_FIRSTDCDATAGRAM is only relevant for first frame.

Any idea?

Also, EC_MAXLRWDATA - EC_FIRSTDCDATAGRAM , EC_FIRSTDCDATAGRAM is only relevant for first frame.

The code before was/is taking EC_FIRSTDCDATAGRAM into account on all segments, I can refactor and change that if you're confident it doesn't need this.

I wonder if there are a clever way to split a "big" segment? I did a sample slave with 4B Out/1540B In.
The logic will create 3 frames when 2 frames easily is enough, but sometimes the clever logic comes with a cost in complexity to maintain and test.

The code right now will pack a segment with as many slaves as will fit, it should be easy to extend that for this use case.

Also, EC_MAXLRWDATA - EC_FIRSTDCDATAGRAM , EC_FIRSTDCDATAGRAM is only relevant for first frame.

The code before was/is taking EC_FIRSTDCDATAGRAM into account on all segments, I can refactor and change that if you're confident it doesn't need this.

I wonder if there are a clever way to split a "big" segment? I did a sample slave with 4B Out/1540B In.
The logic will create 3 frames when 2 frames easily is enough, but sometimes the clever logic comes with a cost in complexity to maintain and test.

The code right now will pack a segment with as many slaves as will fit, it should be easy to extend that for this use case.

We can keep EC_FIRSTDCDATAGRAM to keep it simple, how would you otherwise be able to calculate an adjustment of WKC correctly? You don't know what segment it will belong to.

The logic should avoid splitting slave data between segments when possible.

The code is refactored so the wkc is cleared an incremented all within the config mapping functions rather than spread across separate functions as before. It will also pack large slave data into the current segment to potentially avoid using additional segments.

The logic should avoid splitting slave data between segments when possible.

I set it up this way and I'll agree this is just a preference for what "feels right". For what it's worth, the code can be shorter (and network traffic optimized) by always packing as tightly as possible and I can't find any spec that says a slave must be handled in a single PDU. The downside being that wireshark dumps might be harder to decipher with more optimized packing.

a cost in complexity to maintain and test

You mentioned maintenance and testing yesterday, how do you test this code? Is there a way to use SOES and run some sort of automated testing?

The code is refactored so the wkc is cleared an incremented all within the config mapping functions rather than spread across separate functions as before. It will also pack large slave data into the current segment to potentially avoid using additional segments.

The logic should avoid splitting slave data between segments when possible.

I set it up this way and I'll agree this is just a preference for what "feels right". For what it's worth, the code can be shorter (and network traffic optimized) by always packing as tightly as possible and I can't find any spec that says a slave must be handled in a single PDU. The downside being that wireshark dumps might be harder to decipher with more optimized packing.

The rationale is to make Wireshark dumps as easy to dissect as possible.

a cost in complexity to maintain and test

You mentioned maintenance and testing yesterday, how do you test this code? Is there a way to use SOES and run some sort of automated testing?

I've created two SOES slaves on XMC48 that is 4B Out/1540B In and 1540B Out/4B In. I run manual tests.

Thank you for the info, let me know if there is anything else you want me to do.

It looks good! Nice work.
I'd like to do some more regression testing on bigger networks but I'll be out of office until next Monday, thank you for understanding it'll take a few more days before merge.

…WDATA bytes. Move wkc calcs into relevant config map functions.

…ment

…Tsociety#907) * Fix single device inputs/outputs overruning segment size * Refactor to pack segments tightly. * Allow segments 2-64 to be EC_MAXLRWDATA bytes. * Move wkc calcs into relevant config map functions. * Only split slave data between segments when it is larger than one segment * Only update wkc when we include data for the current device

fix single device inputs/outputs overruning segment size

0b87f56

update wkc calc

6a84071

nakarlsson closed this Apr 25, 2025

nakarlsson reopened this Apr 25, 2025

nakarlsson reviewed Apr 28, 2025

View reviewed changes

soem/ethercatconfig.c Outdated Show resolved Hide resolved

nakarlsson reviewed Apr 28, 2025

View reviewed changes

ddi-kkugler added 3 commits April 28, 2025 14:33

refactor to pack segments tightly. allow segments 2-64 to be EC_MAXLR…

4bacdb0

…WDATA bytes. Move wkc calcs into relevant config map functions.

only split slave data between segments when it is larger than one seg…

a440d6f

…ment

only update wkc when we include data for the current device

f487702

nakarlsson merged commit 66a0bf1 into OpenEtherCATsociety:master May 5, 2025
3 of 5 checks passed

ddi-kkugler deleted the segmented_io_fix branch May 7, 2025 17:15

hefloryd mentioned this pull request Jul 7, 2025

Bug with first slave with many PDOs and DC enabled. #243

Closed

Conversation

ddi-kkugler commented Apr 14, 2025

Uh oh!

ddi-kkugler commented Apr 16, 2025

Uh oh!

ddi-kkugler commented Apr 21, 2025

Uh oh!

nakarlsson commented Apr 23, 2025

Uh oh!

ddi-kkugler commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ddi-kkugler commented Apr 24, 2025

Uh oh!

ddi-kkugler commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nakarlsson commented Apr 24, 2025

Uh oh!

ddi-kkugler commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nakarlsson commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ddi-kkugler commented Apr 25, 2025

Uh oh!

ddi-kkugler commented Apr 28, 2025

Uh oh!

Uh oh!

nakarlsson Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddi-kkugler Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

nakarlsson Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

ddi-kkugler Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nakarlsson Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

ddi-kkugler Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

nakarlsson Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ddi-kkugler commented Apr 24, 2025 •

edited

Loading

ddi-kkugler commented Apr 24, 2025 •

edited

Loading

ddi-kkugler commented Apr 25, 2025 •

edited

Loading

nakarlsson commented Apr 25, 2025 •

edited

Loading

nakarlsson Apr 28, 2025 •

edited

Loading

ddi-kkugler Apr 29, 2025 •

edited

Loading