Added monitor_t class

[montoyaedu]

Hello, I have added a monitor_t class to zmq.hpp for supporting zmq socket monitoring.

It should work this way.

    zmq::socket_t* s = new zmq::socket_t (*context, ZMQ_REQ);
    s->connect("address");
    s->init_monitor("inproc://monitor.initiator", ZMQ_EVENT_ALL);

then s->monitor(zmq::monitor_t*) should be launched within a separated thread.

zmq::monitor_t* must implement the following methods.

class monitor_t
{
    public:
    virtual void on_event_connected(const char *addr_) = 0;
    virtual void on_event_connect_delayed(const char *addr_) = 0;
    virtual void on_event_connect_retried(const char *addr_) = 0;
    virtual void on_event_listening(const char *addr_) = 0;
    virtual void on_event_bind_failed(const char *addr_) = 0;
    virtual void on_event_accepted(const char *addr_) = 0;
    virtual void on_event_accept_failed(const char *addr_) = 0;
    virtual void on_event_closed(const char *addr_) = 0;
    virtual void on_event_close_failed(const char *addr_) = 0;
    virtual void on_event_disconnected(const char *addr_) = 0;
    virtual void on_event_unknown(int event) = 0;
};

Please let me know if you think it could be a good idea or not.

Regards,

Edu

[ricnewton]

Hi Edu,

Why add this code to socket_t and force the overhead of the monitor stuff on everyone even if they are not using it? Wouldn't it be better to move all this code to monitor_t, you can pass in the socket and context in monitor_t's constructor.

Also, making all the virtual functions in monitor_t pure virtual means anyone using it has to implement all functions, even if they are only interested in one event. Why not provide default empty methods so they can just override the ones they want?

You should also add a virtual destructor to monitor_t.

Ric.

[montoyaedu]

Hi Richard,

Thanks for your comments. I think you are right. I will try to modify
sources following your advice.

Thanks again for spending your time reviewing this code.

Edu.

On Tue, Mar 12, 2013 at 10:35 AM, Richard Newton
notifications@github.comwrote:

Hi Edu,

Why add this code to socket_t and force the overhead of the monitor stuff
on everyone even if they are not using it? Wouldn't it be better to move
all this code to monitor_t, you can pass in the socket and context in
monitor_t's constructor.

Also, making all the virtual functions in monitor_t pure virtual means
anyone using it has to implement all functions, even if they are only
interested in one event. Why not provide default empty methods so they can
just override the ones they want?

You should also add a virtual destructor to monitor_t.

Ric.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/15#issuecomment-14765960
.

[montoyaedu]

@hintjens

Hi Pieter,

I think this merge is not ok with the latest version on libzmq.

changes have been made to zmq_event_t structure by @guidog

https://github.com/montoyaedu/libzmq/commit/21eeb03b6cab29e5f4cb60b195272314f6375678

https://github.com/montoyaedu/libzmq/commit/b0b8ab27c5a3734aa2e9103a61e1a850e14d7dd3

btw. I don't know why the following line was eliminated. I think it could be useful.

char *addr;          // endpoint affected as c string

please verify the following commit if you really want to merge.

https://github.com/montoyaedu/cppzmq/commit/824d510bf2559c58a67ec8ff88152966b7e6305c

I also changed some things adviced by @ricnewton

The only part i did not implement was:

"Why add this code to socket_t and force the overhead of the monitor stuff on everyone even if they are not using it? Wouldn't it be better to move all this code to monitor_t, you can pass in the socket and context in monitor_t's constructor."

Right now I dont have so much time for it. please feel free to implement it.

Regards,

Edu.

[hintjens]

I'll wait for someone who is using C++ to make a pull request with this patch. Thanks!

[guidog]

Hi all!

@montoyaedu
Regarding the address part:
It has been removed because it's is now in it's own message part.

And the monitoring stuff needs to be in the socket because the events are generated there.
The above mentioned monitor_t class is not even in the canonical ØMQ interface, cppzmq is
just another binding.

Hope that clarifies some things.

Cheers
Guido

[montoyaedu]

@guidog
Hi Guido. It is clear now. Thanks.

@ricnewton
Hi Richard, Based on Guido's reply do you think it is ok for the cpp binding to keep monitoring stuff within socket?

@hintjens
Pieter, Ok. I will do a pull request once I fix some conflicts I found, so please don't merge the commits above.

Edu.

[ricnewton]

Well, I still would move the majority out the code out into monitor_t, only the init_monitor bit needs to be in socket_t and event that could be moved to monitor_t if we made it a friend in socket_t.

I'll try and re-work it how I have in mind then let you guys decide which way is better.

[montoyaedu]

@hintjens
@ricnewton
just added #21

[montoyaedu]

@ricnewton

Usage my be something like the following snippet?

    zmq::socket_t* s = new zmq::socket_t (*context, ZMQ_REQ);
    s->connect("address");
    zmq::monitor_t* mon = new zmq::monitor_t(*context, s);
    mon->init_monitor("inproc://monitor.initiator", ZMQ_EVENT_ALL);