OpenSimulationInterface / osi-visualizer

**This repository is abandoned and looking for a maintainer.** A tool to visualize OSI data from two different sources. Data can be stored in files and replayed afterwards, or validated using a Python interface.
Other
39 stars 25 forks source link

win32 work-around #31

Closed MEpping closed 6 years ago

MEpping commented 6 years ago

Fixes some issues in Windows & MSVC Winsock redefinition issue no unistd.h _USE_MATH_DEFINES

monishgogri commented 6 years ago

Alternatively you may also use MinGW (with QT Creator) to remove the issues of unistd.h and _USE_MATH_DEFINES.

MEpping commented 6 years ago

The second commit adds a point cloud to the visualizer. The attached screenshot shows data from the KITTI dataset. osipointcloud_screenshot

monishgogri commented 6 years ago

Hello, I want to check this on windows, could you please share how you run it on windows? And which tools/software are you using? Thanking you.

MEpping commented 6 years ago

Sure, thanks for your interest. I compile it in version 15.8.2 of Visual Studio for x64 platform, my OS is Win 10. I wrote a small component for RTMaps to send the KITTI data and I use unreleased versions of dSPACE software to send simulated point clouds. The transmission is just via protobuf & ZMQ, so any server that sends sensor data to the visualizer would work.

KK-MS commented 6 years ago

@MEpping Thank you for the changes, looks good. Good work!

Trying to be ignorant on the code and functionality, I observe that there are some config changes, will this affect the execution on Linux? or is it mandatory for it to be windows compatible?

Could you send a pull request, with the changes sufficient enough to run on Windows and Linux?

MEpping commented 6 years ago

I intended to commit only the changes that caused compiler warnings (double to float with possible data loss..) but I now realize I also changed some default values of the sensor field of view. I guess you are referring to those changes? They are not manditory for Windows and will must likely not affect Linux. They are reasonable to show the point cloud, however, because otherwise the field of view will be extremely narrow by default.

KK-MS commented 6 years ago

You are right, Knowing minimal changes will help us to make sure that the new changes added can be easily reviewed for the compatibility on both OS. My team member @monishgogri is also working with the same goal. We would like to collaborative work with you to include your changes.

MEpping commented 6 years ago

I split up my pull request, hope this helps in the review process. I'm new to git and apologize for the confusion.

monishgogri commented 6 years ago

Hello, thank you for splitting the pull requests, it will be easier to maintain both the features. I will try to run the code on Windows and Linux OS, and once successful we can push it to the main branch.

monishgogri commented 6 years ago

When I am trying to run the code on MS Visual studio 2017 (15.8.3) on windows 10 OS (x64), I am getting the following error: LNK1104 cannot open file 'open-simulation-interface\open_simulation_interface.lib' I am new to Visual Studio, and it would be really helpful when you could tell me which point I am missing, since you might have also faced similar errors. (I am using QT 5.11.1, FMI 2.0.3, and protobuf 2.6.1) Also please tell me if you made any changes in the Cmakelists file. Hoping to get this running soon on Visual studio, Thanking you. image

MEpping commented 6 years ago

You need to build the subproject open_simulation_interface. I link the static lib (see open_simulation_interface_static.lib).

pmai commented 6 years ago

@monishgogri The subproject open_simulation_interface should be built automatically by VS when using the CMake generated projects. Did you populate the submodules after cloning/checking out the repo via git submodule init && git submodule update (or the equivalent in your git client)?

pmai commented 6 years ago

@MEpping the CI build currently fails because there seems to be a reference to pointcloud stuff still left in the changeset?

MEpping commented 6 years ago

@pmai It seems to me the check failure is due to the open-simulation-interface sub-project? I did not change anything over there... The changes for the point cloud have been reverted in this pull request.

monishgogri commented 6 years ago

Hello, thanks for your inputs. @MEpping yes, now I edited the paths in Linker->Input->Additional Depend. @pmai yes I have populated the OpenSimulationInterface directory too. But now I am facing a new error, that the FMI library related symbols are unresolved. Did you face any similar issue? If yes how did you tackle it and did you make any additional changes from Visual studio? (I compiled the FMILibrary 2.0.3 from https://jmodelica.org/ using Visual studio.) Thanks in advance. image

MEpping commented 6 years ago

yes, I remember that I ran into similar problems: make sure you link against the relevant libraries (which you compiled). Unfortunately they are not found automatically by cmake. I think fmilib_shared.lib is one of them.

monishgogri commented 6 years ago

@MEpping thanks for your reply. The problem was that I had compiled FMILibrary for x32 and OSI Visualizer was getting compiled for x64. The problem was fixed when I recompiled the FMILibraries for x64. I was able to successfully generate the osi-visualizer.exe although still some tweaks need to be made. 2018-09-26_18h56_20

MEpping commented 6 years ago

@monishgogri That's good to hear. Please keep me posted on your progress!

MEpping commented 6 years ago

Btw, @monishgogri, looks to me like the resources folder is not found in your screenshot. You can copy it to the working directory of the visualizer executable.

monishgogri commented 6 years ago

@MEpping thanks for the inputs. BTW, do get rid of manually copy pasting the Resources folder to the Debug folder, you can add the following in the CMakeLists.txt file: if(WIN32) message("Adding the path to the OSI Visualizer directory.") add_definitions(-DOVDIR="${CMAKE_CURRENT_SOURCE_DIR}/") else() add_definitions(-DOVDIR="./") endif()

And change the line 54 of src/appconfig.cpp to: , srcPath_(OVDIR)

Did you test your build with the sample file: example_sensor_view.txt provided here ( https://github.com/OpenSimulationInterface/osi-visualizer/releases )? when I try running the file, I get the following error: 2018-09-27_10h19_05 Did you face something similar?

MEpping commented 6 years ago

@monishgogri No, I didn't try this example data. I just tried and get the same error. It's suspicious that the error message refers to SensorData? I tried my build with a simple test server where I constructed a basic sensor data message with attached ground truth.

pmai commented 6 years ago

Ok, I've tried to debug the CI build issues, and I'm having trouble following the sense of the osiv_winos branch: It seems that all changes up to now in the OpenSimulationInterface/osi-visualizer:osiv_winos branch only seem to add, then delete, then re-add PNG image files, with no other changes, except for removing the required open-simulation-interface submodule. I.e. all the changes talked about in the commit messages from @monishgogri are not contained in the commits at all (they just break the build by removing the open-simulation-interface submodule randomly).

This also causes the actual Win32 changes commited by @MEpping to fail to build, because they are based on the broken osiv_winos branch.

I'll cherry-pick the actual file changes on top of a new branch from master, and create a new PR for that, in order to clean up this mess (I'll also clean up the erroneous double submodule definition for osi-sensor-model-packaging introduced some time ago in master on the way...).

Note that I'm not really maintaining osi-visualizer (because we do not use it), but this seems like simple git breakage that should be gotten rid of.

pmai commented 6 years ago

Closing this PR, please take a look at #33 whether this works for you, @MEpping, @monishgogri, and if it does not, please continue work on this basis.