inet-framework / inet

INET Framework for the OMNeT++ discrete event simulator
https://inet.omnetpp.org
Other
441 stars 488 forks source link

Consistent use of EligibilityTimeGate and Ieee8021qAsynchronousShaper #987

Closed haug-den-lucas closed 3 months ago

haug-den-lucas commented 4 months ago

To me the ATS showcase description was quite confusing because it took me a while to figure out that the Ieee8021qAsynchronousShaper is exactly the same as the EligibilityTimeGate.

To this end, I added a comment in the NED file of the Ieee8021qAsynchronousShaper and consistently use the EligibilityTimeGate module in showcase descriptions and configurations.

For the cbsandats showcase I was usure what to do and decided to not change it as it helps here for clarification which queue uses which shaper.

levy commented 4 months ago

It should be the Ieee8021qAsynchronousShaper because using the EligibilityTimeGate is just an implementation detail. In other words, the EligibilityTimeGate module is a generally useful component that may or may not be used to implement the Ieee8021qAsynchronousShaper. The latter is the one that is supposed to implement the standard.

haug-den-lucas commented 4 months ago

Hi, in that case the showcase description and the showcase description and definition are still inconsistent and should then be updated to consistently use one terminology within the TSN showcases.

ATS showcase:

Usage of EligibilityTimeGate: https://github.com/inet-framework/inet/blob/3af5f7e1f1aeb8e17094d2e59d25c6aa8dfc0339/showcases/tsn/trafficshaping/asynchronousshaper/doc/index.rst#L50 https://github.com/inet-framework/inet/blob/3af5f7e1f1aeb8e17094d2e59d25c6aa8dfc0339/showcases/tsn/trafficshaping/asynchronousshaper/doc/index.rst#L199 https://github.com/inet-framework/inet/blob/3af5f7e1f1aeb8e17094d2e59d25c6aa8dfc0339/showcases/tsn/trafficshaping/asynchronousshaper/omnetpp.ini#L87

Usage of Ieee8021qAsynchronousShaper https://github.com/inet-framework/inet/blob/3af5f7e1f1aeb8e17094d2e59d25c6aa8dfc0339/showcases/tsn/trafficshaping/asynchronousshaper/doc/index.rst#L80

ATS and CBS showcase:

Consistently uses Ieee8021qAsynchronousShaper

Unter the hood showcase

Consistently uses EligibilityTimeGate

levy commented 4 months ago

You are right, we should update the showcases to reflect the intentions consistently.

haug-den-lucas commented 4 months ago

I messed up my forked repo, sorry and this was automatically closed. Shall I open an issue for this @levy ?

levy commented 4 months ago

You should open another PR with the suggested changes.