Code review comment for lp:~elachuni/txamqp/heartbeat

Sorry for the slow response. I think adding heartbeat is a very useful feature, thanks for taking the time for implementing it. However, there's a couple of issues:

1 - The virtual host was changed to "/", instead of "localhost". We really need to implement "profiles" or whatever, so we can select some sane defaults for every broker. Until then, I'm not comfortable with introducing a change that's not intrinsic to the problem your branch solves.

2 - Although the code works and is clear, I'd rather use a LoopingCall (http://twistedmatrix.com/documents/8.2.0/api/twisted.internet.task.LoopingCall.html). It's already built into Twisted, so you don't need to implement things like lastSent, lastReceived, etc. I think it would be a bit easier to do something like this (it's just pseudo-python, can't guarantee that it works):

class AMQClient(...):
    # Max unreceived heartbeat frames. The AMQP standard says it's 3.
    MAX_UNSEEN_HEARTBEAT = 3

    def __init__(self, ...):
        ...
        if self.heartbeatInterval > 0:
            self.sendHB = LoopingCall(self.sendHeartbeat)
            self.checkHB = LoopingCall(self.checkHeartbeat)
            d = self.started.wait()
            d.addCallback(lambda _: self.sendHB.start(self.heartbeatInterval, now=False))
            d.addCallback(lambda _: self.checkHB.start(
                self.heartbeatInterval * self.MAX_UNSEEN_HEARTBEAT, now=False))

    def sendHeartbeat(self):
        self.sendFrame(Frame(0, Heartbeat()))

    def checkHeartbeat(self):
        self.checkHB.stop()
        self.transport.loseConnection()

    def processFrame(self, frame):
        ...
        if frame.payload.type != Frame.HEARTBEAT:
            self.sendHB.stop()
            self.sendHB.start(self.heartbeatInterval, now=False)
            ch.dispatch(frame, self.work)
        self.checkHB.stop()
        self.checkHb.start(self.heartbeatInterval * self.MAX_UNSEEN_HEARTBEAT, now=False)

What do you think guys? The changes are minimal and I think they are a bit easier to read.