brianmc95 / OpenCV2X

OMNeT++ V2X simulation framework for ETSI ITS-G5
GNU General Public License v2.0
37 stars 15 forks source link

Error in calculation of “duration ”and “remainingTime ”in LteMacVUeMode4.cc file #24

Open PeiZhonghui opened 3 years ago

PeiZhonghui commented 3 years ago

Hi brianmc95,

I found an error in calculation of “duration ”and “remainingTime ”in LteMacVUeMode4.cc file. image

Here I think the calculation of the "remainingtime" should be the "lteInfo->getDuration()" minus "elapsedTime". For example: When elapsedTime = 1ms and lteInfo->getDuration()=1000ms, after the calculation on line 635, duration=1000ms-1ms = 999ms, and after the calculation on line 637, remainingTime_=1000ms-999ms=1ms=elapsedTime.

Also, there are some problems with the conversion of time units. Line 636 is a conversion in seconds. According to the Settings in operation, the actual running results are as follows: ElapsedTime is usually equal to 0. after the calculation on line 635, duration=1000ms-0ms = 1000ms=1s, and after line 636, dur=1; Because.dbl() is converted in seconds. But lteInfo->getDuration() gets the result in milliseconds. So the final calculation result is: remainingTime_=1000-1=999. This is clearly wrong.

In addition, I wonder what the purpose of the if statement in lines 643-649 is. Is the purpose here as I understand it? For messages of different periods, if the RRI of the existing reserved resources cannot meet the shorter period (or smaller tolerable delay), resource reselection will be performed. I noticed that the "duration" is set to a fixed value of 1000ms in Mode4App.cc in the application layer program. But it is defined in the 3GPP standard as 20ms<=T<=100ms, and it should be changed according to different message periods. For example, when the period of the message is 50ms, its maximum tolerated delay is also 50ms.Is that what "duration" means?

image In line 538-546 of the physical layer file "ltephyvuemode4.cc", the maximum delay is configured again according to the period. Is there a bug in this part of the if statement? What is the purpose of line 538? Is it redundant?

Looking forward to your reply.

brianmc95 commented 3 years ago

Hi @PeiZhonghui,

Sorry for the delay in response.

For the first part on duration I will investigate further today to confirm but I believe you're correct that some of these conversions are currently incorrect and I will incorporate a fix for this in the latest release.

As for 643-649 yes that is exactly the case it is designed such that when the latency requirements cannot be met by the current grant must be broken and a new one scheduled that meets the requirement.

That 1000ms is really just a default so that in experiments you don't run into latency issues unless that's something you want to investigate so changing it is totally valid, ultimately the choice depends on the application requirements you want to mimic. Yes that is the case in the 3GPP standard 20ms<=T<=100ms but it is also in the standard that T will be configured to meet the latency requirements of the application as such this if statement simply managed that i.e. if the latency requirement is > 100 then the period of the message determines T i.e. 50ms or 20ms if you are sub 100ms otherwise 100ms. So this if statement defaults to taking the maximum latency if it is sub 100ms otherwise it takes the correct T for the current period.

I hope this clears things up somewhat and I will get back to you once I have the first issue resolved properly.

Kind Regards, Brian

brianmc95 commented 3 years ago

Hi @PeiZhonghui

So after some investigation I came up with a simple fix based on your comments. Firstly lines 631-637 can be replaced with the following:

receivedTime_ = NOW;
simtime_t elapsedTime = receivedTime_ - lteInfo->getCreationTime();
remainingTime_ = lteInfo->getDuration() - (elapsedTime.dbl() * 1000);

This change ensures the correct calculation of the elapsed time so the remainingTime is fixed in this case. It is also important in further searching of the MAC layer to update the flushHarqBuffers method in the case where no valid MCS is found that if statement needs to change slightly. The start of the if statement remains the same it's just the internal stage where remainingTime_ is changed that has been updated.

if (!foundValidMCS)
{
  // Never found an MCS to satisfy the requirements of the message must regenerate grant
  receivedTime_ = NOW;
  simtime_t elapsedTime = receivedTime_ - lteInfo->getCreationTime();
  remainingTime_ = lteInfo->getDuration() - (elapsedTime.dbl() * 1000);

This change will be incorporated with the latest release of the model which is in progress and includes a few different bug fixes.

If you would like to incorporate these changes to test then please do or if you have any other questions then just let me know.

Kind Regards, Brian

PeiZhonghui commented 3 years ago

Hi @brianmc95

Thank you very much for your reply.

I will use packets with different periods for debugging later.

Best regards, Zhonghui