zeromq / cppzmq

Header-only C++ binding for libzmq
http://www.zeromq.org
MIT License
1.9k stars 757 forks source link

Feature/expose monitor socket for active poller #612

Closed dietelTiMaMi closed 10 months ago

dietelTiMaMi commented 10 months ago

Motivation: zmq::monitor_t in it's current form is inconvenient to use in combination with zmq::active_poller_t the issue is, that the socket isn't available to the user of zmq::monitor_t

Proposal: split zmq::monitor_t::check_event() to move the actual reading and processing of the events into a new protected method zmq::monitor_t::process_event() provide a protected socket_ref for using the monitor_socket for polling in a derived class.

Execution: The proposed changes were done tozmq.hpp A test case demonstrating the desired usage was added to tests/monitor.cpp

By inheriting from zmq::monitor_t you can now implement custom ways of polling for socket events. The modified api is hidden for "rookie" users that might accidentaly call process_event() without polling for changes before use.

dietelTiMaMi commented 10 months ago

Please restart the check, ran on my fork aftrer removing the chrono literals. they are not available in C++11. It seems the Ubuntu 18 runners are no longer existent

dietelTiMaMi commented 10 months ago

@gummif Could you please relaunch the CI tests? in the first run there was some C++14 code that didn't compile in C++11 inside the test that is now fixed.

I also ran the CI in my fork and except for the Ubuntu 18.04 runners that are no longer existent everything went through.

dietelTiMaMi commented 10 months ago

@gummif

Thanks for restarting the workflow. Do you see any problems for merging this? If you would like to have any changes, I will gladly contribute to be able to use it in our project.

gummif commented 10 months ago

Hi I will try to take a look today.

gummif commented 10 months ago

You decide if the comment is worth fixing, otherwise looks good.

coveralls commented 10 months ago

Pull Request Test Coverage Report for Build 6106326776


Changes Missing Coverage Covered Lines Changed/Added Lines %
zmq.hpp 35 70 50.0%
<!-- Total: 35 70 50.0% -->
Totals Coverage Status
Change from base Build 5678621379: 0.7%
Covered Lines: 854
Relevant Lines: 982

💛 - Coveralls