-
Notifications
You must be signed in to change notification settings - Fork 121
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
XEP-0353: add disco (based upon PR #1141) #1142
Conversation
7d47d84
to
d476f62
Compare
d476f62
to
b418a7e
Compare
sorry for the delay! I don't think this is the way to go, the lack of a disco is intentional. |
The lack of disco already made (as far as I know) Conversations and others implement a disco even though it is not specified in the xep. @iNPUTmice could you clarify the usecases of having a disco a bit? First hand implementers experience could help a lot here. |
As far as I know, both, Dino and Conversations implement a disco and I will implement one for Monal, too. That makes this spec another spec that does not reflect reality, which is sad. |
@fippo Why is the lack of disco intentional? Please elaborate! |
The main use-cases of 0353 is sending a message to a bare JID in order for it to be forked to all available resources. If you discover JMI support what do you gain? You'd still send to the bare JID and can't use the information returned from the information discovered from any available resource (and you can not make decisions about devices which appear to be offline but would get a push notification) |
Related issue: snikket-im/snikket-server#86 Based on the number of "why can't I see a call button?" support questions I see almost daily from a range of clients, I am inclined to lean towards less discovery and more graceful failure handling when JMI responds with an error or not at all. That said, with the advent of long-lived device identifiers (new SASL2/bind2), I also consider this a candidate for a feature we could eventually advertise on the account rather than on individual devices (i.e. when the server knows a JMI-capable client has an active push registration). |
@mwild1 Yes, I don't like Conversation's behavior here, too. What I want to accomplish with Monal is: show a warning that the call might not reach the receiver if not at least one device can be found having that disco feature and don't show that warning at all, if at least one device can be found. @fippo For that reason I'd appreciate to have a disco. I'm even going one step further: I will add such disco to Monal even if not specified in the XEP. And yes, surely with a big fat warning not to prevent calls if the disco can't be found because of temporarily offline devices that could still be woken up via push etc.. |
On paper I agree with you. However JMI will usually not respond with an error. It doesn’t even respond with an acknowledgment that some device has started ringing.¹ So if you have a perfectly good device online right now that supports direct call initialization but not JMI your timeout for going from JMI to let’s do direct IQ has to be fairly high, unnecessarily prolonging the time the device (that only supports direct init) starts ringing. That’s what the disco feature in Siskin, Dino and Conversations is for. In a world where JMI was a quasi requirement for jingle calls I would agree with you that always sending JMI is the right move. But this wasn’t the case in 2020 when Conversations implemented calls and with the namespace bump in JMI and competing XEPs that are dedicated for call initialization I’m not sure we will get there in the short term. I think it's also somewhat ironic that Disco is a thing in XMPP and the one time when Conversations uses that to actually show or hide UI everyone hates it. In general I don’t think disco is the wrong move. We just need to figure out how to make disco work with offline devices (account disco?) instead of finding work arounds for each XEP that may want to rely on disco. P.S.: Somewhat related (but not really). On one of the projects I work on we more or less got rid of JMI in favor of direct init for speed reasons. (less round trips; the bulk of the session information is already at the callers end when it starts ringing etc) P.P.S.: I’m not even advocating for adding a disco feature. I’m just explaining why Conversations introduced it almost three years ago. If you consider supports old version of JMI as does not support JMI then Monal right now is in the exact same situation as Conversations was three years ago. Plenty of client support A/V calls. Virtually none of them support JMI. It would be a shame if Monal (or Conversations back then) artificially restricts what calls to call by making JMI a requirement. ¹: Conversations sneakily tries to fake this with delivery receipts. |
@iNPUTmice Using jingle directly exhibits bad UX, especially on iOS where it can happen far more often that a client is offline (e.g. not even having a hibernated XEP-0198 session) making direct jingle fail. But do you think, JMI 0.4 will be implemented by clients any time soon? |
I think this can be closed now. Many clients (at least Siskin, Dino, Conversations and Monal) will do disco. The XEP just does not reflect this reality, but that's fine by me. |
@jcbrand This is based upon my second PR (#1141) and adds a new revision (0.4.1)
This adds the disco feature suggested by Daniel Gultsch on the standards@ list.