[Merge] lp:~mardy/location-service/last-known-position-15.04 into lp:location-service/15.04

Thomas Voß thomas.voss at canonical.com
Thu Nov 19 09:32:59 UTC 2015


Review: Needs Fixing

LGTM.

Diff comments:

> 
> === modified file 'src/location_service/com/ubuntu/location/engine.h'
> --- src/location_service/com/ubuntu/location/engine.h	2015-04-23 14:48:44 +0000
> +++ src/location_service/com/ubuntu/location/engine.h	2015-11-19 09:21:10 +0000
> @@ -127,7 +127,7 @@
>      struct Updates
>      {
>          /** The current best known reference location */
> -        core::Property<Update<Position>> reference_location{};

I have given this some thought, and I think the naming scheme last_known_* makes more sense than reference.

> +        core::Property<Optional<Update<Position>>> reference_location{};
>          /** The current best known velocity estimate. */
>          core::Property<Update<Velocity>> reference_velocity{};

We probably want to have the reference velocity/heading optional, too.

>          /** The current best known heading estimate. */
> 
> === modified file 'src/location_service/com/ubuntu/location/service/implementation.cpp'
> --- src/location_service/com/ubuntu/location/service/implementation.cpp	2014-08-13 13:08:22 +0000
> +++ src/location_service/com/ubuntu/location/service/implementation.cpp	2015-11-19 09:21:10 +0000
> @@ -149,5 +152,21 @@
>          new ProxyProvider{provider_selection}
>      };
>  
> -    return session::Interface::Ptr{new culs::session::Implementation(proxy_provider)};
> +    session::Interface::Ptr session_iface{new session::Implementation(proxy_provider)};
> +    std::weak_ptr<session::Interface> session_weak{session_iface};
> +    session_iface->updates().position_status.changed().connect([this, session_weak](const session::Interface::Updates::Status& status)
> +    {
> +        cul::Optional<cul::Update<cul::Position>> last_known_position =
> +            configuration.engine->updates.reference_location.get();
> +        if (last_known_position &&
> +            status == culs::session::Interface::Updates::Status::enabled)
> +        {
> +            /* Immediately send the last known position to the client */

Mind changing to //? Me being picky here.

> +            if (auto session_iface = session_weak.lock())
> +            {
> +                session_iface->updates().position = last_known_position.get();
> +            }
> +        }
> +    });
> +    return session_iface;
>  }


-- 
https://code.launchpad.net/~mardy/location-service/last-known-position-15.04/+merge/277960
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service/15.04.



More information about the Ubuntu-reviews mailing list