iOS support #3150
iOS support #3150
Conversation
35434c0 to
e354218
Compare
| @@ -101,16 +101,34 @@ NODE_PLATFORM_TARGET := --target_arch=arm64 --target_platform=linux | |||
| TOOLCHAIN_LDD_OPTS := --root $(RASPBIAN)/ | |||
| endif # ($(TARGET),rpi3-armv8) | |||
|
|
|||
| ifeq ($(TARGET),ios-simulator) | |||
There was a problem hiding this comment.
Does that means we have different binaries built for the simulator vs for real hardware ?
Can we run tests on those real hardware binary from the simulator?
There was a problem hiding this comment.
Hardware is ARM64, simulator is x86_64, so no, can't run tests on real hardware binary.
There was a problem hiding this comment.
They have no way to run ARM64 simulator? Bad, it means we'll require more dedicated hardware and baby-sitting setup :/
There was a problem hiding this comment.
I don't think we have the capacity to handle real hardware tests for iOS.
There was a problem hiding this comment.
Do you mean the capacity at iOS simulator level, or our capacity in term of workload? I'm concerned that we have nothing exercizing at all, even basically, the library on ARM64 context, because it's very likely that we will break things somehow :/
There was a problem hiding this comment.
I mean in terms of the workload. New iPhones + new iOS released every year, Apple is famous for breaking compatibility and forcing developers to change their workflows frequently, plus none of us are iOS developers so everything takes twice as long to setup. I agree it's unfortunate to not have ARM64 test coverage.
native_client/swift/deepspeech_ios_test/deepspeech_ios_test/AppDelegate.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM but:
- no tests yet? maybe you plan to add them after?
- some files seems dead now
- a little cleanup on the tensorflow build
ifmess withcasemight be easier to read - since we are building the workers, maybe we can simplify the base setup and limit ourselves to just depending on XCode ?
Correct.
Yeah, thanks for the comments, will clean those up.
Will take a look.
Yeah, that'd be nice. Filed #3169 |
|
|
||
| if [ "${build_android_arm}" = "yes" ]; then | ||
| case "$1" in | ||
| "--cpu") |
There was a problem hiding this comment.
Can you file a bug for later? I think it could be a good idea we decouple more here and have a list like: "--linux-cpu"|"--windows-cpu" etc. But that miight require quite some move love, so let's not hold your PR on that
| @@ -30,7 +30,8 @@ public enum DeepSpeechError: Error { | |||
| case failCreateSess(errorCode: Int32) | |||
| case failCreateModel(errorCode: Int32) | |||
|
|
|||
| // Additional case for invalid error codes, should never happen unless the user has mixed header and binary versions | |||
| // Additional case for invalid error codes, should never happen unless the | |||
There was a problem hiding this comment.
Nice documenting. Do you know if we are ready to run swift-backed doc against readthedocs?
There was a problem hiding this comment.
Unfortunately there is no Swift support for our Sphinx version. So it'll be a bit more complicated.
There was a problem hiding this comment.
But I think it's supported by Doxygen, so we can use the existing support for that :)
There was a problem hiding this comment.
I looked for that too and couldn't find any Doxygen support. Do you have a link?
There was a problem hiding this comment.
Nope, I just saw a few people here and there that seemed to have it processing, as well as some apple github like https://github.com/apple/swift/blob/master/docs/doxygen.header
There was a problem hiding this comment.
That is for Doxygen docs of the Swift compiler, which is implemented in C++.
No description provided.