end2endzone / win32Arduino

win32Arduino is a windows/linux implementation of many arduino functions to allow an arduino library developer to unit test code outside of the arduino platform.
MIT License
2 stars 2 forks source link

wrong syntax for variables on windows #33

Closed sidey79 closed 6 years ago

sidey79 commented 6 years ago

When defining the environment Variables on a windows system, it is not correct to get the information from the variable wirh a $

Instead the following setting:

RAPIDASSIST_HOME=$WIN32ARDUINO_HOME/lib/RapidAssist

does not work on a windows system, it must be

RAPIDASSIST_HOME=%WIN32ARDUINO_HOME%/lib/RapidAssist

end2endzone commented 6 years ago

You are right.

However, the exact command on windows should be set RAPIDASSIST_HOME=%WIN32ARDUINO_HOME%\lib\RapidAssist. Same with Linux (AFAIK), the exact command should be export RAPIDASSIST_HOME=$WIN32ARDUINO_HOME/lib/RapidAssist. In these steps, I didn't wanted to go into details because the commands for creating environment variables are different from Linux and windows. (Use of \ instead of / or export vs set or use of % instead of $, etc)

1) One option would be to use a cmake-like notation. For example, RAPIDASSIST_HOME=${WIN32ARDUINO_HOME}/lib/RapidAssist. This way, most people would understand that they need to adjust for their own platform.

2) Another option is to specify the build steps for Windows AND Linux. Something like

# Build Steps #

## Windows ##

blah blah

`set RAPIDASSIST_HOME=%WIN32ARDUINO_HOME%\lib\RapidAssist`

## Linux ##

blah blah

`export RAPIDASSIST_HOME=$WIN32ARDUINO_HOME/lib/RapidAssist`

I usually don't like this option because I don't like repeating the same information. It is also too easy to make mistakes. You run a search-and-replace and forget that Linux has this or that character which won't be picked by the replace. Or you copy-and-paste a section from one to the other and forget you need to adjust for Linux.

However, in this scenario, it may be better/appropriate to create a Windows and a Linux build section.

Which option do you prefer ?

end2endzone commented 6 years ago

I am fine with both option and open to suggestion. You can also propose a 3rd solution :) Which option do you prefer ? I will gladly make the change.

sidey79 commented 6 years ago

The problem is correctly understood.

When i was reading the docs, i was thinking, that this is a copy past job on windows, because i was here in the docs: https://github.com/end2endzone/win32Arduino/tree/v2.2.2#2-create-your-test-project-manually-with-visual-studio

So in my opinion, visual studio runs only on windows and the examples works for that situation. And what is also not absolutly clear to be is, the thing with cmake and visual studio.

I got it, that all the libs compile via cmake, so compiled stuff is on the build folders for release and i did it also for debug.

But my visual studio project doesn't compile after this step. I had to load the rapidassist and win32arduino project files into my solution and add them as dependent libs. That wasn't clear to me and im still not sure, if i need a cmake --build . --config Release because i think visual studio does the compile job.

So having a documentation walk through, it shoud focus on the use case, which was 1. visual studio in my case. My second case is doing automatic tests on travis-ci or another build environment. What do you think to make different install docs for the different use cases?

end2endzone commented 6 years ago

I had to load the rapidassist and win32arduino project files into my solution and add them as dependent libs.

This manual step should not be required. If you properly updated your git submodules and configured your environment variables, the solution should already contains rapidassist, gtest and win32arduino project files.

After running cmake .. command, you should have the win32Arduino.sln solution file in your build directory. If you open the solution file with Visual Studio, the rapidassist, gtest and gtest_main should already be part of the project. (Remember, you do not require to build RapidAssist and GTest before building win32Arduino).

In other words, this is what you should see: win32arduino vs solution explorer

The cmake --build . --config Release compiles the code on the command line. When executed, CMake will internally call Visual Studio compiler (without the GUI) to build the solution. If you prefer to compile with the Visual Studio IDE (aka GUI), then the equivalent task would be the following: 1) Click on win32Arduino.sln to open Visual Studio IDE. 2) Click on menu Build and select Build Solution. (The F7 key also launches the compiler) 3) Wait for the compilation to complete. 4) Exit Visual Studio.

You must do one or the other.

end2endzone commented 6 years ago

To clarify the compile using cmake --build . --config Release or Visual Studio IDE issue on windows, I plan to change the documentation from:

run 'cmake --build . --config Release' or open 'win32Arduino.sln' with Visual Studio.

to

run 'cmake --build . --config Release' to compile the code on the command line. Note that you can also compile the code by opening the 'win32Arduino.sln' solution file with Visual Studio and selecting 'Build\Build Solution' option in the top menu bar.
sidey79 commented 6 years ago

I need to check win32Arduino.sln, Bit not clear why i should compile this.

I have a project and want to test this project using win32arduino .

I have a additional project with my test code. All in the same solution.

At what point i am different from your view?

sidey79 commented 6 years ago

Checked win32arduino.sln and it looks good as yours and compiles.

But, what is the idea, how to implement them in a existing solution?

sidey79 commented 6 years ago

I've added your win32arduino.sln into my existing solution following this guide: https://blogs.msdn.microsoft.com/habibh/2009/06/25/walkthrough-adding-an-existing-visual-studio-solution-to-another-solution/

There i saw, that the solution does not contain anything from the googletest suite.

This needs a Build option, to be included: cmake -D WIN32ARDUINO_BUILD_TEST=ON ..

Now i have new problems which needs to be investigated :(

end2endzone commented 6 years ago

So having a documentation walk through, it shoud focus on the use case, which was 1. visual studio in my case. My second case is doing automatic tests on travis-ci or another build environment. What do you think to make different install docs for the different use cases?

I am not certain that we should document that much the use case. I mean, yes more documentation is always better but the benefit may not be that big. My point is that this whole build process has changed for the next release (2.3.x). I learned a lot about CMake during the last 6-12 months and I think that I have removed all this confusion by sticking to 'CMake's best practices'.

The list of changes includes the following:

The new procedure is already working but certainly lacks documentation (as always).

If you want and have the time, you could make the required modifications for the use cases for v2.2.x and I will gladly include them in the documentation. But IMHO, I find it hard to justify putting this kind of effort knowing the build/install/usage process has moved on.

end2endzone commented 6 years ago

I pushed a new commit (bfd8c1198ac52861ef09811b8d6589cc60018f97). This is a documentation update that should clarify new build instructions for Windows and Linux and remove ambiguity about submodules and dependencies. If should fix issue #21, issue #32 and issue #33. Please tell me if this change has resolved your issue and I will close it.

sidey79 commented 6 years ago

I think spending time on docs for 2.2.x dosen't makes much sense if you change it with 2.3.x that way.

It's may better to spend my time on that Version.

Regaeding cmake i am a absolut noob.

end2endzone commented 6 years ago

Regaeding cmake i am a absolut noob

Feel free to open a new issue against 2.3.0 documentation. Things should be easier to understand with 2.3.0.

sidey79 commented 6 years ago

One last question, is there a branch with 2.3.0 already available?

end2endzone commented 6 years ago

No. Version 2.3.0 is on master. My plan is to stay on master branch for as long as possible. I created branch prerelease-v2.2.x because there was too many commit on master that changed the 'nature' of version 2.2.1. In other words, after tagging 2.2.1 (which was on master), I added new features on master that I didn't wanted in 2.2.2 so I didn't have a choice but to create a branch from tag 2.2.1.