Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.Modelica.Utilities.System.getTime() returns bad year #3143

Closed
adrpo opened this issue Oct 10, 2019 · 11 comments · Fixed by #3144
Closed

.Modelica.Utilities.System.getTime() returns bad year #3143

adrpo opened this issue Oct 10, 2019 · 11 comments · Fixed by #3144
Assignees
Labels
bug Critical/severe issue L: C-Sources Issue addresses Modelica/Resources/C-Sources V: 3.2.2 Issue originates in MSL v3.2.2 (and is not present in earlier releases)
Milestone

Comments

@adrpo
Copy link
Collaborator

adrpo commented Oct 10, 2019

Running this model:

model GetTime
  Integer ms;
  Integer sec;
  Integer min;
  Integer hour;
  Integer mon;
  Integer year;
algorithm
  (ms, sec, min, hour, mon, year) := .Modelica.Utilities.System.getTime();
  .Modelica.Utilities.Streams.print("ms:" + String(ms) + "\n");
  .Modelica.Utilities.Streams.print("sec:" + String(sec) + "\n");
  .Modelica.Utilities.Streams.print("min:" + String(min) + "\n");
  .Modelica.Utilities.Streams.print("hour:" + String(hour) + "\n");
  .Modelica.Utilities.Streams.print("mon:" + String(mon) + "\n");
  .Modelica.Utilities.Streams.print("year:" + String(year) + "\n");
end GetTime;

with OpenModelica on Windows will give you year=10 and on Linux will give you year=9.
With Dymola 2019 on Windows ill give you year=10.

The code for the external C is here:
https://github.com/modelica/ModelicaStandardLibrary/blob/master/Modelica/Resources/C-Sources/ModelicaInternal.c#L1264
I haven't looked in detail into it, maybe somebody can spot the problem.

@adrpo
Copy link
Collaborator Author

adrpo commented Oct 10, 2019

@HansOlsson
Copy link
Contributor

The call should be:
(ms, sec, min, hour, mday, mon, year) := .Modelica.Utilities.System.getTime();

So that you get day of the month as well. I don't know why Linux results are from last month :-)

@beutlich beutlich added L: C-Sources Issue addresses Modelica/Resources/C-Sources question Unexplained or undecided issue labels Oct 10, 2019
@adrpo
Copy link
Collaborator Author

adrpo commented Oct 10, 2019

Yeah, I read the doc wrong. But still the issue with the month on Linux remains.
Correct model should be:

model GetTime
  Integer ms;
  Integer sec;
  Integer min;
  Integer hour;
  Integer mday;
  Integer mon;
  Integer year;
algorithm
  (ms, sec, min, hour, mday, mon, year) := .Modelica.Utilities.System.getTime();
  .Modelica.Utilities.Streams.print("ms:" + String(ms) + "\n");
  .Modelica.Utilities.Streams.print("sec:" + String(sec) + "\n");
  .Modelica.Utilities.Streams.print("min:" + String(min) + "\n");
  .Modelica.Utilities.Streams.print("hour:" + String(hour) + "\n");
  .Modelica.Utilities.Streams.print("mday:" + String(mday) + "\n");
  .Modelica.Utilities.Streams.print("mon:" + String(mon) + "\n");
  .Modelica.Utilities.Streams.print("year:" + String(year) + "\n");
end GetTime;

@beutlich
Copy link
Member

beutlich commented Oct 10, 2019

Hm, only for Win32 we increment the month

tlocal->tm_mon = tlocal->tm_mon + 1; /* Correct for month starting at 1 */

@beutlich beutlich added bug Critical/severe issue and removed question Unexplained or undecided issue labels Oct 10, 2019
@beutlich beutlich assigned beutlich and unassigned HansOlsson Oct 10, 2019
@beutlich beutlich added this to the MSL4.0.0 milestone Oct 10, 2019
@HansOlsson
Copy link
Contributor

Hm, only for Win32 we increment the month

  ModelicaStandardLibrary/Modelica/Resources/C-Sources/ModelicaInternal.c


     Line 1306
  in
  caaf8b1





    
      
       tlocal->tm_mon  = tlocal->tm_mon + 1;     /* Correct for month starting at 1 */

And the same for the year as well, which will be even more obvious.

@beutlich
Copy link
Member

beutlich commented Oct 10, 2019

Yes, it was wrong from the beginning when I copied it from Modelica_Noise for MSL v3.2.2

https://github.com/DLR-SR/Noise/blob/891468c5c25c39f49c785bbdb853a386a4d0e889/Modelica_Noise%201.0%20Beta.1/Resources/Include/ModelicaRandom.c#L208-L243

@beutlich beutlich added the V: 3.2.2 Issue originates in MSL v3.2.2 (and is not present in earlier releases) label Oct 10, 2019
beutlich added a commit to beutlich/ModelicaStandardLibrary that referenced this issue Oct 10, 2019
@sjoelund
Copy link
Member

I guess the problem with easily assigning to the bad value is due to not returning enumeration (or wrapping it into a record).

@HansOlsson
Copy link
Contributor

I guess the problem with easily assigning to the bad value is due to not returning enumeration (or wrapping it into a record).

Wrapping it into a record seems the most logical.

Alternatively I think that the risk would have been less if we had used the same order as IS 8601 and reversed the output order to return year, month, ... (but changing it now would be too confusing).

@adrpo
Copy link
Collaborator Author

adrpo commented Oct 11, 2019

Could we change it to a record for 4.x?

@sjoelund
Copy link
Member

I think there might be a possibility to rename getTime() to Internal.getTime() or something with the same interface. And then create a new getTime() that works by calling that function and creating a record on top of it.

@beutlich
Copy link
Member

beutlich commented Dec 2, 2019

I think there might be a possibility to rename getTime() to Internal.getTime() or something with the same interface. And then create a new getTime() that works by calling that function and creating a record on top of it.

This is addressed by #3247.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Critical/severe issue L: C-Sources Issue addresses Modelica/Resources/C-Sources V: 3.2.2 Issue originates in MSL v3.2.2 (and is not present in earlier releases)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants