geolib: datasets: warn when not installed; update install script; launch SIGINT when not installed #778
Conversation
TSC21
force-pushed
the
geolib_mandat
branch
14 times, most recently
from
bda4a98
to
50f3677
Compare
TSC21
force-pushed
the
geolib_mandat
branch
10 times, most recently
from
c0438d2
to
9c68469
Compare
TSC21
force-pushed
the
geolib_mandat
branch
2 times, most recently
from
e355777
to
8437678
Compare
TSC21
force-pushed
the
geolib_mandat
branch
2 times, most recently
from
8935ef4
to
1cbe623
Compare
|
I'm going to leave the docker container update for another PR. This as is seems enough to fix travis and at the same time add the dataset load as mandatory. @vooon can you review please? |
TSC21
changed the title
| else() | ||
| message(STATUS "Magnetic Field model datasets found in: " ${GEOGRAPHICLIB_MAGNETIC_PATH_}/magnetic) | ||
| set(GEOGRAPHICLIB_MAGNETIC_PATH ${GEOGRAPHICLIB_MAGNETIC_PATH_}/magnetic) | ||
| endif() |
There was a problem hiding this comment.
NO! Wrong! You should check in runtime. UAS or so should try to load needed datasets and responds error log if it fails.
There was a problem hiding this comment.
Wrong? Wrong why? Check the all PR and you will see that's also done. This is a check during build time that warns people that they basically did not fully followed the installation instructions. A SIGINT is launched when that happens. What I can also do is verbose the SIGINT with a info msg so that people run the install script. Besides that, this is perfectly acceptable IMO and doesn't harm anyone.
.travis.yml
Outdated
| @@ -26,7 +31,7 @@ before_install: | |||
| #- sudo sh -c "echo \"deb http://packages.ros.org/ros-shadow-fixed/ubuntu $ROS_CI_DESKTOP main\" > /etc/apt/sources.list.d/ros-latest.list" | |||
| - wget http://packages.ros.org/ros.key -O - | sudo apt-key add - | |||
| # Update python 2.7 version | |||
| - sudo apt-add-repository ppa:fkrull/deadsnakes-python2.7 -y | |||
| - sudo apt-add-repository ppa:fkrull/deadsnakes -y | |||
There was a problem hiding this comment.
ppa deprecatead : use https://launchpad.net/~deadsnakes/+archive/ubuntu/ppa
Why do you need it btw ?
There was a problem hiding this comment.
Problems in Travis. But thanks for the updated version
mavros/src/lib/uas_data.cpp
Outdated
| ROS_ERROR_STREAM("UAS: GeographicLib exception: " << e.what()); | ||
| signal(SIGINT, term); | ||
| } |
There was a problem hiding this comment.
No! Why you set signal callback here? Just call ROS_ABORT().
There was a problem hiding this comment.
Well I wasn't sure what was the best approach. I know that SIGINT can be intercepted besides allowing proper debugging. I was thinking that we could add the info on the SIGINT stating that the problem is due to not having the dataset installed. Though I suppose that can also be handled on the ROS_ERROR_STREAM above.
There was a problem hiding this comment.
But that it not what you wrote in code.
Code:
- try to load egm96-5
- on error:
- Log error
- Set SIGINT signal handler.
man signal
No description provided.