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.

Sign up for GitHub

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

Removing queues from UI #79

Merged
merged 9 commits into from
Oct 21, 2023
Merged

Removing queues from UI #79

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8a1e93a
Cleanup to remove queues in UI component
Garfeild Oct 11, 2023
4274266
Fixes for queue
Garfeild Oct 14, 2023
344e953
Removing queue around DeviceService call in MiniSim
Garfeild Oct 15, 2023
f9298c7
Adding own queue for DeviceService
Garfeild Oct 15, 2023
d9f5179
Removing redundant check
Garfeild Oct 16, 2023
098c169
Reverting main queue usage in Menu
Garfeild Oct 17, 2023
6e00759
Addressing PR comments
Garfeild Oct 21, 2023
e551fdd
Removing DispatchQueue.main in Menu
Garfeild Oct 21, 2023
5a1a15e
Bumping service level for DeviceService inner queue
Garfeild Oct 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions MiniSim/AppleScript Commands/LaunchDeviceCommand.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,11 @@ class LaunchDeviceCommand: NSScriptCommand {
return nil
}

DispatchQueue.global(qos: .userInitiated).async {
if device.platform == .android {
try? DeviceService.launchDevice(name: device.name)
} else {
try? DeviceService.launchDevice(uuid: device.ID ?? "")
}
}

DeviceService.launch(device: device) { _ in }
return nil
} catch {
scriptErrorNumber = NSInternalScriptError;
return nil
}

}
}
56 changes: 18 additions & 38 deletions MiniSim/Menu.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,16 @@ class Menu: NSMenu {
}

func getDevices() {
DispatchQueue.global(qos: .userInitiated).async {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code was the root cause of everything being not in the main queue. And that's the most important change in the PR. New DeviceService.getAllDevices executes callback closure on the main queue by default. This allows us to remove any DispatchQueue.main calls in the Menu.

Copy link
Owner

Choose a reason for hiding this comment

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

Okay, great thanks for the explanation!

do {
var devicesArray: [Device] = []
if UserDefaults.standard.enableiOSSimulators {
try devicesArray.append(contentsOf: DeviceService.getIOSDevices())
}
if UserDefaults.standard.enableAndroidEmulators && UserDefaults.standard.androidHome != nil {
try devicesArray.append(contentsOf: DeviceService.getAndroidDevices())
}
self.devices = devicesArray
} catch {
let userDefaults = UserDefaults.standard
DeviceService.getAllDevices(
android: userDefaults.enableAndroidEmulators && userDefaults.androidHome != nil,
iOS: userDefaults.enableiOSSimulators
) { devices, error in
if let error = error {
okwasniewski marked this conversation as resolved.
Show resolved Hide resolved
NSAlert.showError(message: error.localizedDescription)
return
}
self.devices = devices
}
}

Expand Down Expand Up @@ -75,22 +72,14 @@ class Menu: NSMenu {

@objc private func deviceItemClick(_ sender: NSMenuItem) {
guard let device = getDeviceByName(name: sender.title) else { return }
guard let tag = DeviceMenuItem(rawValue: sender.tag) else { return }

if device.booted {
DeviceService.focusDevice(device)
return
}

DispatchQueue.global(qos: .userInitiated).async {
do {
switch tag {
case .launchAndroid:
try DeviceService.launchDevice(name: device.name)
case .launchIOS:
try DeviceService.launchDevice(uuid: device.ID ?? "")
}
} catch {
DeviceService.launch(device: device) { error in
if let error = error {
okwasniewski marked this conversation as resolved.
Show resolved Hide resolved
NSAlert.showError(message: error.localizedDescription)
}
}
Expand All @@ -116,9 +105,7 @@ class Menu: NSMenu {
private func assignKeyEquivalent(devices: [NSMenuItem]) {
for (index, item) in devices.enumerated() {
if index > maxKeyEquivalent {
DispatchQueue.main.async {
item.keyEquivalent = ""
}
item.keyEquivalent = ""
okwasniewski marked this conversation as resolved.
Show resolved Hide resolved
continue
}

Expand All @@ -128,10 +115,8 @@ class Menu: NSMenu {
continue
}

DispatchQueue.main.async {
if self.items.contains(item) {
item.keyEquivalent = keyEquivalent
}
if self.items.contains(item) {
Copy link
Owner

Choose a reason for hiding this comment

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

Here we are still missing the DispatchQueue.main

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added it back here.

Copy link
Owner

Choose a reason for hiding this comment

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

We need to check if item exists inside of the DispatchQueue.main.async call as it might not exists when we get to executing the inner closure on the main thread. Can you please for now bring back the old approach? And later on we can refactor it with Async/Await.

item.keyEquivalent = keyEquivalent
}
}
}
Expand All @@ -142,11 +127,9 @@ class Menu: NSMenu {
for (index, device) in sortedDevices.enumerated() {
let isAndroid = device.platform == .android
if let itemIndex = items.firstIndex(where: { $0.title == device.displayName }) {
DispatchQueue.main.async { [self] in
let item = self.items.get(at: itemIndex)
item?.state = device.booted ? .on : .off
item?.submenu = isAndroid ? populateAndroidSubMenu(booted: device.booted) : populateIOSSubMenu(booted: device.booted)
}
let item = self.items.get(at: itemIndex)
item?.state = device.booted ? .on : .off
item?.submenu = isAndroid ? populateAndroidSubMenu(booted: device.booted) : populateIOSSubMenu(booted: device.booted)
continue
}

Expand All @@ -162,11 +145,8 @@ class Menu: NSMenu {
menuItem.submenu = isAndroid ? populateAndroidSubMenu(booted: device.booted) : populateIOSSubMenu(booted: device.booted)
menuItem.state = device.booted ? .on : .off

DispatchQueue.main.async {
let iosDevicesCount = self.devices.filter({ $0.platform == .ios }).count
self.safeInsertItem(menuItem, at: isAndroid && UserDefaults.standard.enableiOSSimulators ? (isFirst ? index : iosDevicesCount) + 3 : 1)
}

let iosDevicesCount = self.devices.filter({ $0.platform == .ios }).count
Copy link
Owner

Choose a reason for hiding this comment

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

Also here we are missing the jump back to the main thread. You could add the DispatchQueue.main.async to the safeInsertItem method.

self.safeInsertItem(menuItem, at: isAndroid && UserDefaults.standard.enableiOSSimulators ? (isFirst ? index : iosDevicesCount) + 3 : 1)
}
}

Expand Down
15 changes: 7 additions & 8 deletions MiniSim/MiniSim.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,19 +148,18 @@ class MiniSim: NSObject {
if !shouldDelete {
return
}
DispatchQueue.global(qos: .userInitiated).async {
do {
let amountCleared = try DeviceService.clearDerivedData()
UNUserNotificationCenter.showNotification(title: "Derived data has been cleared!", body: "Removed \(amountCleared) of data")
NotificationCenter.default.post(name: .commandDidSucceed, object: nil)
} catch {
NSAlert.showError(message: error.localizedDescription)

DeviceService.clearDerivedData() { amountCleared, error in
guard error == nil else {
NSAlert.showError(message: error?.localizedDescription ?? "Failed to clear derived data.")
return
}
UNUserNotificationCenter.showNotification(title: "Derived data has been cleared!", body: "Removed \(amountCleared) of data")
NotificationCenter.default.post(name: .commandDidSucceed, object: nil)
}
default:
break
}

}
}

Expand Down
90 changes: 78 additions & 12 deletions MiniSim/Service/DeviceService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,11 @@ import AppKit
import UserNotifications

protocol DeviceServiceProtocol {
static func launchDevice(uuid: String) throws
static func getIOSDevices() throws -> [Device]
static func checkXcodeSetup() -> Bool
static func deleteSimulator(uuid: String) throws
static func clearDerivedData() throws -> String
static func handleiOSAction(device: Device, commandTag: IOSSubMenuItem, itemName: String)

static func launchDevice(name: String, additionalArguments: [String]) throws
static func toggleA11y(device: Device) throws
static func getAndroidDevices() throws -> [Device]
static func sendText(device: Device, text: String) throws
Expand All @@ -34,6 +31,7 @@ protocol DeviceServiceProtocol {

class DeviceService: DeviceServiceProtocol {

private static let queue = DispatchQueue(label: "com.MiniSim.DeviceService", qos: .utility)
okwasniewski marked this conversation as resolved.
Show resolved Hide resolved
private static let deviceBootedError = "Unable to boot device in current state: Booted"

private static let derivedDataLocation = "~/Library/Developer/Xcode/DerivedData"
Expand Down Expand Up @@ -93,7 +91,7 @@ class DeviceService: DeviceServiceProtocol {
}

static func focusDevice(_ device: Device) {
DispatchQueue.global(qos: .userInitiated).async {
queue.async {

let runningApps = NSWorkspace.shared.runningApplications.filter({$0.activationPolicy == .regular})

Expand Down Expand Up @@ -155,6 +153,63 @@ class DeviceService: DeviceServiceProtocol {
UNUserNotificationCenter.showNotification(title: title, body: message)
NotificationCenter.default.post(name: .commandDidSucceed, object: nil)
}

static func getAllDevices(
android: Bool,
iOS: Bool,
completionQueue: DispatchQueue = .main,
completion: @escaping ([Device], Error?) -> ()
) {
queue.async {
do {
var devicesArray: [Device] = []

if android {
try devicesArray.append(contentsOf: getAndroidDevices())
}

if iOS {
try devicesArray.append(contentsOf: getIOSDevices())
}

completionQueue.async {
completion(devicesArray, nil)
}
} catch {
completionQueue.async {
completion([], error)
}
}
}
}

private static func launch(device: Device) throws {
switch device.platform {
case .ios:
try launchDevice(uuid: device.ID ?? "")
case .android:
try launchDevice(name: device.name)
}
}

static func launch(device: Device, completionQueue: DispatchQueue = .main, completion: @escaping (Error?) -> Void) {
self.queue.async {
do {
try self.launch(device: device)
completionQueue.async {
completion(nil)
}
}
catch {
guard error.localizedDescription.contains(deviceBootedError) else {
return
}
completionQueue.async {
completion(error)
}
}
}
}
}

// MARK: iOS Methods
Expand Down Expand Up @@ -182,10 +237,21 @@ extension DeviceService {
return devices
}

static func clearDerivedData() throws -> String {
let amountCleared = try? shellOut(to: "du -sh \(derivedDataLocation)").match(###"\d+\.?\d+\w+"###).first?.first
try shellOut(to: "rm -rf \(derivedDataLocation)")
return amountCleared ?? ""
static func clearDerivedData(completionQueue: DispatchQueue = .main, completion: @escaping (String, Error?) -> Void) {
self.queue.async {
do {
let amountCleared = try? shellOut(to: "du -sh \(derivedDataLocation)").match(###"\d+\.?\d+\w+"###).first?.first
try shellOut(to: "rm -rf \(derivedDataLocation)")
completionQueue.async {
completion(amountCleared ?? "", nil)
}
}
catch {
completionQueue.async {
completion("", error)
}
}
}
}

static func getIOSDevices() throws -> [Device] {
Expand All @@ -206,7 +272,7 @@ extension DeviceService {
}
}

static func launchDevice(uuid: String) throws {
private static func launchDevice(uuid: String) throws {
do {
try self.launchSimulatorApp(uuid: uuid)
try shellOut(to: ProcessPaths.xcrun.rawValue, arguments: ["simctl", "boot", uuid])
Expand Down Expand Up @@ -237,7 +303,7 @@ extension DeviceService {
if !NSAlert.showQuestionDialog(title: "Are you sure?", message: "Are you sure you want to delete this Simulator?") {
return
}
DispatchQueue.global(qos: .userInitiated).async {
queue.async {
do {
try DeviceService.deleteSimulator(uuid: deviceID)
DeviceService.showSuccessMessage(title: "Simulator deleted!", message: deviceID)
Expand All @@ -250,7 +316,7 @@ extension DeviceService {
guard let command = DeviceService.getCustomCommand(platform: .ios, commandName: itemName) else {
return
}
DispatchQueue.global(qos: .userInitiated).async {
queue.async {
do {
try DeviceService.runCustomCommand(device, command: command)
} catch {
Expand All @@ -267,7 +333,7 @@ extension DeviceService {

// MARK: Android Methods
extension DeviceService {
static func launchDevice(name: String, additionalArguments: [String] = []) throws {
private static func launchDevice(name: String, additionalArguments: [String] = []) throws {
let emulatorPath = try ADB.getEmulatorPath()
var arguments = ["@\(name)"]
let formattedArguments = additionalArguments.filter({ !$0.isEmpty }).map {
Expand Down