Merge lp://staging/~thomas-voss/location-service/fix-1462664 into lp://staging/location-service/15.04

Proposed by Thomas Voß
Status: Merged
Approved by: Alberto Mardegan
Approved revision: 199
Merged at revision: 199
Proposed branch: lp://staging/~thomas-voss/location-service/fix-1462664
Merge into: lp://staging/location-service/15.04
Diff against target: 93 lines (+25/-26)
2 files modified
src/location_service/com/ubuntu/location/service/daemon.cpp (+4/-23)
src/location_service/com/ubuntu/location/service/session/skeleton.cpp (+21/-3)
To merge this branch: bzr merge lp://staging/~thomas-voss/location-service/fix-1462664
Reviewer Review Type Date Requested Status
Alberto Mardegan (community) Approve
Review via email: mp+275537@code.staging.launchpad.net

This proposal supersedes a proposal from 2015-10-22.

Commit message

Handle responses of clients to updates asynchronously.
Rely on dummy::ConnectivityManager as harvesting is disabled anyway.

Description of the change

Handle responses of clients to updates asynchronously.
Rely on dummy::ConnectivityManager as harvesting is disabled anyway.

To post a comment you must log in.
Revision history for this message
Alberto Mardegan (mardy) wrote : Posted in a previous version of this proposal

All the messages say "Failed to communicate position update to client: ", but the last two could say "heading" and "velocity", respectively. Just in case we want to be able to tell apart the cases in the logs.

Which I don't think is really important, that's why I'm approving this as it is anyway.

review: Approve
Revision history for this message
Alberto Mardegan (mardy) wrote : Posted in a previous version of this proposal

But, how does this fixes the linked issues? The methods where already invoked asynchronously; this just adds error logging (which is good).

Revision history for this message
Thomas Voß (thomas-voss) wrote : Posted in a previous version of this proposal

The previous *async* methods returned a std::future<core::dbus::Result<..>>, which tries to synchronize its state on destruction. Please also note that I'm proposing this early to enable testing easily.

Revision history for this message
Alberto Mardegan (mardy) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches