Skip to content
This repository has been archived by the owner on Feb 21, 2021. It is now read-only.

[visualStates] get cmake variables and use in the code and solves #904 #905

Merged
merged 3 commits into from
Sep 14, 2017

Conversation

okanasik
Copy link
Contributor

This pull request solves following problems

  • a new class is created to get install prefix path from cmake variables
  • after moving default install prefix to /opt/jderobot some library links are broken in cpp template
  • resource locator cannot find glade files since they are not in global glade directory.

… a c++ class, fix broken library path after moving install prefix to /opt/jderobot
@okanasik okanasik changed the title get cmake variables and use in the code and solves #904 [visualStates] get cmake variables and use in the code and solves #904 Sep 12, 2017
@lr-morales
Copy link
Member

Good job.
Not only should solve (I didn't test this to confirm it) the intended issues, but also #485 is partially fixed too.

As I said, I didn't test this PR and maybe I'm wrong, but as CMake is told to generate cmakevars.hpp from cmakevars.hpp.in, is the inclusion of an already generated file unintended?

Regards

@okanasik
Copy link
Contributor Author

@lr-morales yes I have intentionally included the generated file, but if this is not a good practice, I can remove. What do you think?

@lr-morales
Copy link
Member

lr-morales commented Sep 12, 2017

In my opinion, you maybe should remove it as it could mislead someone when looking the code at first sight and as there have been some issues in the past regarding file generation because some residual generated files were left in the clean source folder: CMake was supposed to generate and take a new file but it used the residual one.

Did you left it for any special reason? If it was as an example, maybe renaming to something else would also be a good option.

@okanasik
Copy link
Contributor Author

Ok. Then, it will be better if I remove the file. Thanks.

@aitormf
Copy link
Collaborator

aitormf commented Sep 12, 2017

Hi @okanasik ,
with this changes the project doesn't compile:

as configuring the file, it is saved in build, the compiler is unable to find it.

/home/kasillas77/git/test/okan2/src/tools/visualStates/generate.cpp:24:25: fatal error: cmakevars.hpp: file or directory doesn't exist
 #include "cmakevars.hpp"
                         ^
compilation terminated.

Look at this cmakelist to see how to solve it

@okanasik
Copy link
Contributor Author

okanasik commented Sep 13, 2017

Thanks for testing. I build in the current directory that is why i did not notice. It should be fixed with the last commit 7e40320

@aitormf aitormf merged commit ffbc791 into JdeRobot:master Sep 14, 2017
@aitormf
Copy link
Collaborator

aitormf commented Sep 14, 2017

great job @okanasik !!!

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

Successfully merging this pull request may close these issues.

3 participants