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

/ping command #11

Open
wants to merge 3 commits into
base: torchat_py
Choose a base branch
from
Open

/ping command #11

wants to merge 3 commits into from

Conversation

RubenRocha
Copy link

I've added a /ping command to test latency between 2 people.
How it works is:

  • When you type /ping it sends them a message containing "/ping " + a 6 character unique id
  • They get that /ping request and send you a /PONG + the unique id
  • You check the sent time for /ping and the received time for /PONG and subtract them(Received time - send time) then you divide by 2(So instead of getting the time for 2 messages(You sending to him, and he sending /pong back to you, you get the time for 1 message).

@Someguy123
Copy link

I've tested this with Ruben, and the feature is highly useful.
I'd love to see this natively in torchat.

@meh
Copy link

meh commented Jun 7, 2012

I sincerely don't like abuse of the protocol like this, it would be better to implement it as a protocol extension.

@prof7bit
Copy link
Owner

I am not ignoring this pull request ;-)

Generally the idea to have commands that do other things than only sending IM is good (/ping or /whatever) using the notation / is also ok (because a GUI like pidgin will recognize that it is a special command) but I am not yet sure where to properly and generically implement the handling of any such command and also not yet sure about whether the ping command should work exactly like you have proposed.

Since you brought up this idea I am thinking about it. Only the timing is a little suboptimal: I was really hoping I would not receive pull requests with big feature additions for 0.9.9 anymore and could instead implement all new stuff directly in TorChat 2.0, but I am not progressing as fast as I wanted to, it is quite some work and also currently I also need to reverse engineer a lot of libpurple related stuff (my first purple plugun and they have API docs but no general overview document).

I will not close or reject this pull request, I will leave it open to remind me that I need to decide about it eventually.

@meh
Copy link

meh commented Jun 17, 2012

@prof7bit I think it's either protocol extensions for every command (which would easily become an infinite list, so it's not reallya good choice), or a single packet to handle all commands that contains the name of the command and the data encoded in JSON or something similar.

command ping {"id": "d2082fac-88ca-4673-b057-8d3d65f0f3fe"}

This still grants the simpleness in parsing the general protocol but leaves client that want to support commands more freedom in what kind of data can be sent with the commands.

The only risk would be clients abusing the packet and implementing features that would be better suited as protocol extensions rather than commands.

I think we (as in the various torchat implementors) should work and think together about what protocol extensions would be useful.

I'm already writing a prototype for the latency extension and also working on a distributed addressbook extension that came up when brainstorming with the jTorchat guys.

Just my 2¢.

@ghost ghost assigned prof7bit Jun 17, 2012
@prof7bit
Copy link
Owner

On , "meh."
[email protected]
wrote:

@prof7bit I think it's either protocol extensions for every command
(which would easily become an infinite list, so it's not reallya good
choice), or a single packet to handle all commands that contains the name
of the command and the data encoded in JSON or something similar.

We don't need son for the data. TorChat packet payload may be arbitrary
binary data and for simple things (like a ping) that can be expressed in
plain ascii with space as delimiter it should just be an ascii string
delimited with as many spaces as necessary. Two different ways to encode
stuff in the same protocol does not look right to me.

@meh
Copy link

meh commented Jun 17, 2012

We don't need son for the data. TorChat packet payload may be arbitrary
binary data and for simple things (like a ping) that can be expressed in
plain ascii with space as delimiter it should just be an ascii string
delimited with as many spaces as necessary. Two different ways to encode
stuff in the same protocol does not look right to me.

The point would just be to avoid every implementor having their own
binary interface for passing data more complex than a string list.

But whatever, it's probably just better to come up with stable
protocol extensions than having random commands that change behavior
depending on the client.

@prof7bit
Copy link
Owner

On , "meh."
[email protected]
wrote:

But whatever, it's probably just better to come up with stable

protocol extensions than having random commands that change behavior

depending on the client.

Yes. And for these commands there need to be an additional interface
between GUI and core, an API where the GUI can pass commands to the core
and an API where the core can report the results back to the GUI, it should
not somehow abuse the API for sending and receiving IM.

Pidgin for example will detect such "/" commands and communicate them to
and from the protocol plugin with separate functions and callbacks, not via
the send_im() API. So I will implement such an API in the TorChat 2 core
anyways. And then we can think about what commands actually make sense
and how the core should react. this could also be things like joining a
group chat, things that can also be initiated via the GUI and other things.

@meh
Copy link

meh commented Jun 26, 2012

I wrote the latency protocol extension, already implemented in ruby-torchat.

Don't think it will need anything else, any feedback?

@panczypani panczypani mentioned this pull request Dec 18, 2012
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.

4 participants