Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: add is_connected interface #97

Merged
merged 1 commit into from
Sep 29, 2017
Merged

core: add is_connected interface #97

merged 1 commit into from
Sep 29, 2017

Conversation

julianoes
Copy link
Collaborator

This adds a sync interface for the connection status of a device. It is
basically the same information as also covered by the async methods
on_discover and on_timeout.

Closes #56.

This adds a sync interface for the connection status of a device. It is
basically the same information as also covered by the async methods
on_discover and on_timeout.
bool is_connected() const;

/**
* @brief Returns true if a device is currently connected.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be "Returns true if the specified device is currently connected."

@hamishwillee
Copy link
Collaborator

This allows you to test from a DroneCore object whether you have exactly one object connected or if a particular UUID is connected. It does what it says.

  1. When/why would you want to check you have just one device connected?
  2. I can imagine you might want to test whether a call to Device &device() will succeed but this API won't do that since it returns false if you have multiple connected devices.
  3. The API to get the connection status for a particular UUID should be useful, because we could choose to not get handles to disconnected objects and monitor the connection API to start handling them as needed.

My problem with this is that I don't know if it allows us to sensibly handle the case where we disconnect and reconnect (both with 1 and multiple devices). We need to work out how we would do that in order to decide whether connection information needs to be testable from the Device itself, and whether we need to be able to register for connection/timeout events from the Device itself. Just FYI, added #99 for that example.

@julianoes
Copy link
Collaborator Author

  1. When/why would you want to check you have just one device connected?

Whenever you only expect one device, so most of the time I'd say.

  1. I can imagine you might want to test whether a call to Device &device() will succeed but this API won't do that since it returns false if you have multiple connected devices.

I agree that device() and is_connected() are not behaving the same way since device() returns the first device for the case of multiple connected devices. This makes me wonder if we should go back to the discussion whether device() should return a pointer which can be nullptr. That could make it consistent.

@hamishwillee
Copy link
Collaborator

When/why would you want to check you have just one device connected?

Whenever you only expect one device, so most of the time I'd say.

If I know I'm only ever going to get one device, test seems a little pointless.

@julianoes
Copy link
Collaborator Author

If I know I'm only ever going to get one device, test seems a little pointless.

Just to be sure and to know it is connected.

@julianoes
Copy link
Collaborator Author

I'm merging this for now. We can change it again later.

@julianoes julianoes merged commit 8291e49 into develop Sep 29, 2017
@hamishwillee
Copy link
Collaborator

OK. Thanks!

@julianoes julianoes deleted the add-is-connected branch October 9, 2017 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants