[Merge] lp:~mandel/location-service/testing-connectivity into lp:location-service

Sergio Schvezov sergio.schvezov at canonical.com
Tue Sep 16 12:06:40 UTC 2014


Review: Needs Fixing

Looks good, without known much of the codebase I added some upstart script comments

Diff comments:

> === modified file 'data/CMakeLists.txt'
> --- data/CMakeLists.txt	2014-07-30 14:20:43 +0000
> +++ data/CMakeLists.txt	2014-09-16 12:01:29 +0000
> @@ -30,6 +30,11 @@
>    ubuntu-location-service-trust-stored.conf.in ubuntu-location-service-trust-stored.conf @ONLY
>  )
>  
> +# we are not using @ONLY becuase we want ${CMAKE_INSTALL_BINDIR} to be replaced
> +configure_file(
> +  ubuntu-location-connectivity-tests.conf.in ubuntu-location-connectivity-tests.conf
> +)
> +
>  install(
>    FILES ${CMAKE_CURRENT_BINARY_DIR}/ubuntu-location-service.pc
>    DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig
> @@ -54,3 +59,8 @@
>    FILES ${CMAKE_CURRENT_BINARY_DIR}/ubuntu-location-service-trust-stored.conf
>    DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/upstart/sessions/
>  )
> +
> +install(
> +  FILES ${CMAKE_CURRENT_BINARY_DIR}/ubuntu-location-connectivity-tests.conf
> +  DESTINATION ${CMAKE_INSTALL_BINDIR}/ulc-tests/init
> +)
> 
> === added file 'data/ubuntu-location-connectivity-tests.conf.in'
> --- data/ubuntu-location-connectivity-tests.conf.in	1970-01-01 00:00:00 +0000
> +++ data/ubuntu-location-connectivity-tests.conf.in	2014-09-16 12:01:29 +0000
> @@ -0,0 +1,9 @@
> +description "Daemon used to test the location connectivity API."
> +
> +start on (started dbus and started ofono)

this should be 'manual' or 'start on network-test-session' if you prefer

> +
> +respawn
> +

add a pre-start and post-stop with some timestamps with a clear begin,end/ start,stop

> +console log
> +
> +exec ${CMAKE_INSTALL_BINDIR}/ulc-tests/connectivity
> 
> === modified file 'debian/changelog'
> --- debian/changelog	2014-09-15 20:02:59 +0000
> +++ debian/changelog	2014-09-16 12:01:29 +0000
> @@ -1,3 +1,11 @@
> +location-service (2.1+14.10.20140915-0ubuntu2) UNRELEASED; urgency=medium
> +
> +  * Allow the standalone test to be compiled using the lib in the tree.
> +  * Provide a testing daemon that can be used to test the connectivity
> +    API via an upstart job at boot time.
> +
> + -- Manuel de la Pena <manuel.delapena at canonical.com>  Tue, 16 Sep 2014 13:40:08 +0200
> +
>  location-service (2.1+14.10.20140915-0ubuntu1) utopic; urgency=low
>  
>    [ Loïc Minier ]
> 
> === modified file 'debian/control'
> --- debian/control	2014-08-05 10:27:30 +0000
> +++ debian/control	2014-09-16 12:01:29 +0000
> @@ -123,3 +123,13 @@
>   updates and exporting them over dbus.
>   .
>   Contains documentation for service and client.
> +
> +Package: ubuntu-location-connectivity-tests
> +Architecture: any
> +Depends: ${misc:Depends},
> +         ${shlibs:Depends},
> +Description: location service aggregating position/velocity/heading
> + updates and exporting them over dbus.
> + .
> + Contains a testing daemon that can be used to check the signals
> + received by the connectivity API.
> 
> === added file 'debian/ubuntu-location-connectivity-tests.install'
> --- debian/ubuntu-location-connectivity-tests.install	1970-01-01 00:00:00 +0000
> +++ debian/ubuntu-location-connectivity-tests.install	2014-09-16 12:01:29 +0000
> @@ -0,0 +1,2 @@
> +usr/bin/ulc-tests/init/* /etc/init
> +usr/bin/ulc-tests/bin/*
> 
> === modified file 'examples/CMakeLists.txt'
> --- examples/CMakeLists.txt	2014-05-20 09:45:02 +0000
> +++ examples/CMakeLists.txt	2014-09-16 12:01:29 +0000
> @@ -1,5 +1,6 @@
>  add_subdirectory(service)
> +add_subdirectory(standalone)
>  
>  install(
>    DIRECTORY standalone
> -  DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/${CMAKE_PROJECT_NAME}/examples)
> \ No newline at end of file
> +  DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/${CMAKE_PROJECT_NAME}/examples)
> 
> === added file 'examples/standalone/CMakeLists.txt'
> --- examples/standalone/CMakeLists.txt	1970-01-01 00:00:00 +0000
> +++ examples/standalone/CMakeLists.txt	2014-09-16 12:01:29 +0000
> @@ -0,0 +1,1 @@
> +add_subdirectory(connectivity)
> 
> === modified file 'examples/standalone/connectivity/CMakeLists.txt'
> --- examples/standalone/connectivity/CMakeLists.txt	2014-05-20 09:16:20 +0000
> +++ examples/standalone/connectivity/CMakeLists.txt	2014-09-16 12:01:29 +0000
> @@ -1,7 +1,3 @@
> -cmake_minimum_required(VERSION 2.8)
> -
> -project(connectivity)
> -
>  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
>  
>  find_package(PkgConfig)
> @@ -15,11 +11,10 @@
>    PROCESS_CPP 
>    process-cpp REQUIRED)
>  
> -pkg_check_modules(
> -  LOCATION_CONNECTIVITY 
> -  ubuntu-location-service-connectivity REQUIRED)
> -
>  include_directories(
> +
> +  ${CMAKE_SOURCE_DIR}/include/location_service/com/ubuntu/location/connectivity
> +
>    ${DBUS_CPP_INCLUDE_DIRS}
>    ${PROCESS_CPP_INCLUDE_DIRS}
>    ${LOCATION_CONNECTIVITY_INCLUDE_DIRS})
> @@ -32,10 +27,13 @@
>  target_link_libraries(
>    connectivity
>  
> +  ubuntu-location-service-connectivity
>    ${CMAKE_THREAD_LIBS_INIT}
>  
>    ${DBUS_CPP_LDFLAGS}
> -  ${PROCESS_CPP_LDFLAGS}
> -  ${LOCATION_CONNECTIVITY_LDFLAGS})
> +  ${PROCESS_CPP_LDFLAGS})
>  
> +install(
> +  TARGETS connectivity
> +  DESTINATION ${CMAKE_INSTALL_BINDIR}/ulc-tests/bin)
>  
> 
> === modified file 'examples/standalone/connectivity/connectivity.cpp'
> --- examples/standalone/connectivity/connectivity.cpp	2014-08-22 10:49:52 +0000
> +++ examples/standalone/connectivity/connectivity.cpp	2014-09-16 12:01:29 +0000
> @@ -21,7 +21,7 @@
>  //   (1.) Obtain an instance of location::connectivity::Manager.
>  //   (2.) Connect to the changed signals of the wifi and cell properties and react according to your component's requirements.
>  //   (3.) Bootstrap your own setup by explicitly getting all visible wifis and connected cells.
> -int main(int argc, char** argv)
> +int main(int, char**)
>  {
>      // We catch sig-term to exit cleanly.
>      auto trap = core::posix::trap_signals_for_all_subsequent_threads({core::posix::Signal::sig_term});
> @@ -157,7 +157,7 @@
>          };
>  
>          // Subscribe to last-seen updates.
> -        wifi->last_seen().changed().connect([wp](const std::chrono::system_clock::time_point& tp)
> +        wifi->last_seen().changed().connect([wp](const std::chrono::system_clock::time_point&)
>          {
>              auto sp = wp.lock();
>              if (sp)
> @@ -167,7 +167,7 @@
>          // Subscribe to signal strength updates. Please note that this is not considering
>          // the case of subscribing to already known wifis. We leave this up
>          // to consumers of the api.
> -        wifi->signal_strength().changed().connect([wp](const location::connectivity::WirelessNetwork::SignalStrength& s)
> +        wifi->signal_strength().changed().connect([wp](const location::connectivity::WirelessNetwork::SignalStrength&)
>          {
>              auto sp = wp.lock();
>              if (sp)
> 


-- 
https://code.launchpad.net/~mandel/location-service/testing-connectivity/+merge/234802
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service.



More information about the Ubuntu-reviews mailing list