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

cancelPush() should have target ? #54

Closed
riju opened this issue Sep 23, 2015 · 6 comments
Closed

cancelPush() should have target ? #54

riju opened this issue Sep 23, 2015 · 6 comments

Comments

@riju
Copy link
Collaborator

riju commented Sep 23, 2015

At any moment, there can be a maximum of 2 pushMessage() for an adapter, 1 for tag, 1 for peer.
So correspondingly 2 messageBuffer/NFCMessage, 2 timers, etc.
Presently in cancelPush(), all the instances of pushMessage() are invalidated.
Do you want to add "target" as a parameter to cancelPush() ?

@kenchris
Copy link
Contributor

I think it is fine that it cancels everything for now. Cancelable Promises are on their way and we would want to migrate to using that when that happens (unless that is way in the future).

@zolkis
Copy link
Contributor

zolkis commented Sep 23, 2015

Until cancellable Promises come, we should add a target to cancelPush(). This would also be in line with having 2 separate timers, outboxes, and instances of the algo allowed.

@zolkis
Copy link
Contributor

zolkis commented Sep 23, 2015

Working on it now.

@kenchris
Copy link
Contributor

Please add a NOTE about this probably going away in the future

@kenchris
Copy link
Contributor

@zolkis
Copy link
Contributor

zolkis commented Sep 23, 2015

#50 fixes this.

@zolkis zolkis closed this as completed Sep 24, 2015
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

No branches or pull requests

3 participants