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

Removed testing for delegates in different running modes from iOS Vision Tasks #4583

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

priankakariatyml
Copy link
Collaborator

No description provided.

withCode:MPPTasksErrorCodeFailedPreconditionError
description:[NSString
stringWithFormat:@"This method can only be called if the task is "
@"running in a stream mode and an object of a class "
Copy link
Collaborator

Choose a reason for hiding this comment

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

"an object of a class" is a bit hard to parse. Maybe just "and a delegate is provided in the task options..."

@@ -59,7 +62,20 @@ - (instancetype)initWithCalculatorGraphConfig:(CalculatorGraphConfig)graphConfig
}

- (BOOL)sendPacketMap:(const PacketMap &)packetMap error:(NSError **)error {
if (!_initializedWithPacketsCallback) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This now means that we can initialize a task that will only ever throw errors. Is it possible to revert to the old behavior and throw an error on the initialization?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, but then this whole change is unnecessary right? I mean wouldn't it suffice to just make all the delegate methods @required. Can I close this PR ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably. I think we can have @required methods and tests that the delegate object is set itself (like I believe we do today).

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.

None yet

2 participants