[Merge] lp:~ssweeny/location-service/espoo-delayed-provider.15.04 into lp:location-service/15.04

Thomas Voß thomas.voss at canonical.com
Mon Oct 26 11:08:11 UTC 2015


Review: Needs Fixing

A remark inline about splitting changes between espoo-delayed.provider.15.04 and delayed-provider.15.04.

One other remark: I would prefer a set of more sophisticated test cases that exercise the timing logic. Otherwise, we would have to invest significant manual testing efforts for each landing to ensure correct behavior. Would you mind introducing such tests into this MP?

Diff comments:

> 
> === modified file 'src/location_service/com/ubuntu/location/provider.cpp'
> --- src/location_service/com/ubuntu/location/provider.cpp	2015-10-22 17:19:04 +0000
> +++ src/location_service/com/ubuntu/location/provider.cpp	2015-10-22 17:19:04 +0000
> @@ -142,17 +142,24 @@
>  
>  void cul::Provider::Controller::on_provider_booted(bool was_booted)
>  {
> +    LOG(INFO) << "Provider was booted " << was_booted;
>      if (!was_booted)
>          return;
>  
>      // we need to propagate the state of the provider using the internal counters as references
> -    if (position_updates_counter > 0) {
> +    if (position_updates_counter > 0)

I think we should fold these logging adjustments into delayed_provider.15.04 to clearly separate out the required changes.

> +    {
> +        LOG(INFO) << "Starting position updates";
>          instance.start_position_updates();
>      }
> -    if (heading_updates_counter > 0) {
> +    if (heading_updates_counter > 0)
> +    {
> +        LOG(INFO) << "Starting heading updates";
>          instance.start_heading_updates();
>      }
> -    if (velocity_updates_counter > 0) {
> +    if (velocity_updates_counter > 0)
> +    {
> +        LOG(INFO) << "Starting velocity updates";
>          instance.start_velocity_updates();
>      }
>  }


-- 
https://code.launchpad.net/~ssweeny/location-service/espoo-delayed-provider.15.04/+merge/275424
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service/15.04.



More information about the Ubuntu-reviews mailing list