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

Add dll files and annotation enhancement to include directories #4

Conversation

PMehrfeld
Copy link
Contributor

as discussed in #2 and #3

@tbeu
Copy link
Contributor

tbeu commented Dec 7, 2016

About 871d6fc: I'd like to stick with netcdf 4.3.3.1 pre-built binaries for the following reasons

  • the pre-built binaries require the VS 2013 redistributable, but ncDataReader2.lib and ITI_ncDataReader2.dll are built with VS2010.
  • netcdf pre-built 4.3.3.1 binaries uses hdf5 1.8, wheras netcdf 4.4 uses hdf5 1.10. There are known performance and compatibility issues in hdf 1.10.

Two options (for me) here:

  • Add the pre-built binaries from the netcdf 4.3.3.1 package. Easy to do.
  • Build netcdf 4.4 with hdf5 1.8 on VS 2010. this requires some effort.

So if it is OK for you, I'd simply would add the binaries from the netcdf 4.3.3.1 binary distribution.

@tbeu
Copy link
Contributor

tbeu commented Dec 7, 2016

About f39385a: According to the MLS 3.3 rev1, section 12.9.4 the IncludeDirectory and LibraryDirectory you added are the default ones and need not be specified. Do you really require these annotations for your Modelica tool? Additionally, __iti_dllDirectory is not defined by SimulationX.

@PMehrfeld
Copy link
Contributor Author

I have replaced dll files with the ones from netCDF 4.3.3.1. Still works. But we need these files, otherwise compilation aborts.
You are absolutely right. The suffix ...Directory is not needed in the annotation. I reverted this.

@tbeu
Copy link
Contributor

tbeu commented Dec 7, 2016

Thanks, merged manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants