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

Allow Multiple Devices at Once #83

Closed
wants to merge 1 commit into from
Closed

Allow Multiple Devices at Once #83

wants to merge 1 commit into from

Conversation

jeffsawatzky
Copy link

This is a first stab and supporting multiple devices.
Refs #82
Also fixed some typos.

Note: I AM NOT AN IOS DEVELOPER. :) Please, provide feedback on how to make any of this better.

Also Note: Multiple select on the side bar isn't working...and I don't know how to fix that. But, if I hard code the selectedDevices to just return the list of all self.realDevices the rest of it seems to work.

@@ -1464,7 +1464,7 @@
<rect key="frame" x="0.0" y="0.0" width="86" height="302"/>
<autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/>
<subviews>
<outlineView verticalHuggingPriority="750" allowsExpansionToolTips="YES" columnAutoresizingStyle="lastColumnOnly" selectionHighlightStyle="sourceList" multipleSelection="NO" autosaveColumns="NO" rowHeight="24" rowSizeStyle="medium" viewBased="YES" indentationPerLevel="16" autoresizesOutlineColumn="YES" outlineTableColumn="MzZ-ov-wur" id="7U9-5A-zYV">
<outlineView verticalHuggingPriority="750" allowsExpansionToolTips="YES" columnAutoresizingStyle="lastColumnOnly" selectionHighlightStyle="sourceList" autosaveColumns="NO" rowHeight="28" rowSizeStyle="medium" viewBased="YES" indentationPerLevel="16" autoresizesOutlineColumn="YES" outlineTableColumn="MzZ-ov-wur" id="7U9-5A-zYV">
Copy link
Author

Choose a reason for hiding this comment

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

This doesn't seem to allow multiple selection. I've tried shift+click and cmd+click and neither work.

@@ -90,7 +90,6 @@ extension MapViewController: LocationSpooferDelegate {

// Post the update for the current app status.
NotificationCenter.default.post(name: .StatusChanged, object: self, userInfo: [
"device": spoofer.device,
Copy link
Author

Choose a reason for hiding this comment

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

userInfo["device"] wasn't used in the observer, so to simplify things I just removed this.

@@ -119,7 +118,6 @@ extension MapViewController: LocationSpooferDelegate {

// Update the current application status.
NotificationCenter.default.post(name: .StatusChanged, object: self, userInfo: [
"device": spoofer.device,
Copy link
Author

Choose a reason for hiding this comment

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

userInfo["device"] wasn't used in the observer, so to simplify things I just removed this.

} catch let error {
// Stop the spinner even if an error occured.
self.contentView?.stopSpinner()
self.deviceIsConnectd = false
self.deviceIsConnected = false
Copy link
Author

Choose a reason for hiding this comment

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

Not sure if we should try to disconnect any successfully connected devices here?

@Schlaubischlump
Copy link
Owner

Thank you for the Pull Request ! Selecting multiple devices at the sidebar shouldn't be too hard to implement for me.

The only problem I currently have is that I'm not sure about how this feature should work from the UI side. Currently when you switch the selected device inside LocationSimulator, the spoofed location will be forgotten for the previous device. I plan to change this, so that will work this way:

  1. Select a device and start spoofing
  2. Select a different device => device 1 will pause spoofing
  3. Start spoofing on the second device
  4. Reselect device 1 and continue spoofing where you previously left

If I now allow selecting multiple devices, I'm not sure how I should handle the case where multiple devices are already spoofing. Assume that device 1 and device 2 are currently performing a navigation. I now select both of them simultaneously. What should be shown inside the MapView ? The route of device 1 or the route of device 2 or neither of them ? Should I reset the spoofed location when selecting multiple devices at once ?

Don't get me wrong. This is a feature which I really like and would love to implement, I just don't know how to add it to the UI, without making the process unintuitive.

@jeffsawatzky
Copy link
Author

What is the use case for switching between devices frequently? I was doing that before, but only because I couldn't spoof mulitple devices at the same time.

You could show a dialog to the user with something like:

┌─────────────────────────────────────────────────────────┐
│                                                         │
│  The device you are connecting has a previous location  │
│  saved. What would you like to do?                      │
│                                                         │
│  Choose location:                                       │
│  ┌┐                                                     │
│  └┘ Keep current location                               │
│                                                         │
│  ┌┐                                                     │
│  └┘ Navigate to saved location                          │
│                                                         │
│  ┌┐                                                     │
│  └┘ Transport to saved location                         │
│                                                         │
│                                                         │
│                                                         │
│  Choose devices:                                        │
│  ┌┐                                                     │
│  └┘ Keep all devices connected                          │
│                                                         │
│  ┌┐                                                     │
│  └┘ Disconnect devices currently connected              │
│                                                         │
│                                                         │
│                                                         │
│                   ┌────────────────┐ ┌────────────────┐ │
│                   │                │ │                │ │
│                   │       OK       │ │     Cancel     │ │
│  ┌┐               │                │ │                │ │
│  └┘ Save choice   └────────────────┘ └────────────────┘ │
│                                                         │
└─────────────────────────────────────────────────────────┘

@jeffsawatzky
Copy link
Author

Could also make multi-device support optional, and off by default

@jeffsawatzky jeffsawatzky deleted the mulitple-devices branch November 3, 2021 18:39
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