ptrkstr / Devices

Swift package that contains all devices from https://www.theiphonewiki.com/wiki/Models. Useful when needing to parse machine/device identifier (i.e. iPhone10,1) to device name (iPhone 8).
MIT License
18 stars 4 forks source link

Improve finding Device from Identifier #13

Closed grahamburgsma closed 2 years ago

grahamburgsma commented 2 years ago

Thanks for this library, was the only one I could find that would work for my use case. I'm using this server side to convert device identifiers into human readable model/generation names.

If I could suggestion an improvement, it would be nice to have a more efficient way to find the model for an identifier. Currently I have to search through all the models of iPhone, iPod touch, iPad, iPadAir, iPadMini and iPadPro just to convert an identifier to a generation name. Should probably be searching through all the Mac devices as well since M1/M2 Macs can run iOS apps. Ideally there would be a dictionary lookup to easily and efficiently do this.

Also by having separate models for each type of iPad and Mac, despite the models being exactly the same, will make things more difficult to compare models etc.

ptrkstr commented 2 years ago

Hey @grahamburgsma, thanks for reaching out. To confirm, are you after something like the following being generated?

protocol DeviceType {
    var identifier: String { get }
    var generation: String { get }
    // anything else that is common
}

let allDevices: [String: DeviceType] = ...
let generation = allDevices?["iPhone14,5"]?.generation
grahamburgsma commented 2 years ago

@ptrkstr Thanks for the quick response. Yes something like that would be great!

ptrkstr commented 2 years ago

@grahamburgsma I've just realised that because multiple devices can have the same identifier it would actually be like the following:

let allDevices: [String: [DeviceType]] = ...
let generation = allDevices?["iPhone14,5"]?.first?.generation

They all have the same generation so it wouldn't matter which one you pull out.

grahamburgsma commented 2 years ago

@ptrkstr Ah true, but yes still works in that format too. 👍

ptrkstr commented 2 years ago

@grahamburgsma can you try using the feature/all-devices branch. Usage is:

let generation = Devices.all["iPhone14,5"].first?.generation

There is a remaining issue to resolve where there are multiple identifiers in a key (as that's how the wiki stores it):

"AirPods1,2\nAirPods2,1": [
    Airpod(generation: "AirPods (2nd generation)", aNumber: "A2031 (left AirPod)\nA2032 (right AirPod)\nA1938 (wireless charging case)", bootrom: "Unknown", fccID: "BCG-A2031\nBCG-A2032", internalName: "B288AP", identifier: "AirPods1,2\nAirPods2,1", model: "MRXJ2, MV7N2, MR8U2")
],
"AirPods1,3\nAudio2,1": [
    Airpod(generation: "AirPods (3rd generation)", aNumber: "A2564 (right AirPod?)", bootrom: "Unknown", fccID: "BCG-A2564\nBCG-A2565", internalName: "B688AP", identifier: "AirPods1,3\nAudio2,1", model: "")
],
"AirPods2,2\nAirPodsPro1,1\niProd8,1": [
    Airpod(generation: "AirPods Pro", aNumber: "A2083 (left AirPod)\nA2084 (right AirPod)\nA2190 (AirPods Pro charging case)", bootrom: "Unknown", fccID: "BCG-A2083\nBCG-A2084", internalName: "B298AP", identifier: "AirPods2,2\nAirPodsPro1,1\niProd8,1", model: "MWP22, MLWK3")
],
"AirPodsMax1,1\niProd8,6": [
    Airpod(generation: "AirPods Max", aNumber: "A2096", bootrom: "Unknown", fccID: "BCG-A2096", internalName: "B515AP", identifier: "AirPodsMax1,1\niProd8,6", model: "MGYH3, MGYJ3, MGYL3, MGYM3, MGYN3")
],
grahamburgsma commented 2 years ago

@ptrkstr nice! Curious why you're using OrderedDictionary?

And for the multiple identifiers it looks like splitting by \n could work.

ptrkstr commented 2 years ago

If a regular dictionary is used, every time the all was generated, the ordering would be different and make looking at difs annoying (since dictionary is unordered). An alternative to using OrderedDictionary would be to manually do the sorting and then print but I took the easy route. This doesn't impact the Devices package as it doesn't include Generator.

And for the multiple identifiers it looks like splitting by \n could work.

Yep, will address the new lines, otherwise does it solve your needs?

ptrkstr commented 2 years ago

@grahamburgsma could you confirm the following before I continue:

otherwise does it solve your needs?

grahamburgsma commented 2 years ago

@ptrkstr Sorry I missed that last message. Thanks for the explanation, makes total sense.

And yes, that should work great. Thank you!

ptrkstr commented 2 years ago

Hey @grahamburgsma, I've updated the branch now to separate the new lines. Give it a go and let me know if you're happy with it and I'll merge it.

grahamburgsma commented 2 years ago

@ptrkstr Looks like the all dictionary is at the top level instead of under the Devices namespace, I think that would be preferred.

ptrkstr commented 2 years ago

Good point @grahamburgsma, I initially envisioned it would be accessed via the module name, then all but you're right that it's on the top level and would be accessible without it. I've put it under DeviceList instead of Devices so it doesn't clash with the Module name.

grahamburgsma commented 2 years ago

Looks good to me! 🙌

Although there is this empty file https://github.com/ptrkstr/Devices/blob/feature/all-devices/Sources/Devices/AllDevices.swift.

ptrkstr commented 2 years ago

Great spot @grahamburgsma! Updated and released as 1.1.0, thanks for collaborating on this 🎉