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.

Sign up for GitHub

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

Added getSocketDescriptor() to http::server #140

Merged
merged 1 commit into from
Apr 7, 2016
Merged

Added getSocketDescriptor() to http::server #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

# git master

* [139](https://github.com/HBPVIS/zeq/pull/139):
Added getSocketDescriptor() to http::server
* [138](https://github.com/HBPVIS/zeq/pull/138):
Command line progress monitor

Expand Down
4 changes: 4 additions & 0 deletions tests/http/server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ BOOST_AUTO_TEST_CASE(construction)
BOOST_CHECK_NE( server1.getURI().getHost(), "" );
BOOST_CHECK_NE( server1.getURI().getHost(), "*" );
BOOST_CHECK_NE( server1.getURI().getPort(), 0 );
BOOST_CHECK_NO_THROW( server1.getSocketDescriptor( ));
BOOST_CHECK_GT( server1.getSocketDescriptor(), 0 );

const zeq::URI uri( "tcp://" );
zeq::http::Server server2( uri );
Expand All @@ -124,6 +126,8 @@ BOOST_AUTO_TEST_CASE(construction)

BOOST_CHECK_THROW( zeq::http::Server( server2.getURI( )),
std::runtime_error );
BOOST_CHECK_NO_THROW( server2.getSocketDescriptor( ));
BOOST_CHECK_GT( server1.getSocketDescriptor(), 0 );
}

BOOST_AUTO_TEST_CASE(construction_argv_host_port)
Expand Down
12 changes: 12 additions & 0 deletions zeq/http/server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,18 @@ const servus::URI& Server::getURI() const
return _impl->uri.toServusURI();
}

SocketDescriptor Server::getSocketDescriptor() const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning a low-level fd from the Server seems fishy to me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having access to the socket handle is very useful in conjunction with a QSocketNotifier. It handles the poll() as part of the QEventLoop and emits an activated() QSignal when the socket has new data available. This signal can be connected to a lamba which calls recieive(), so with a few lines of code you have a server which automatically receives without the need for a separate processing thread. I see no easy alternative to that mechanism.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally understand it. But still looks fishy :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a commonly used pattern. You get an opaque handle to poll, but should not use it for something else. ZMQ provides it, X11 ConnectionNumber does, and many more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not disagree, but it feels like there should be extendable layer around it. A socket and poll classes. In C world it is more understandable but in C++ world these can be wrapped nicer.

{
SocketDescriptor fd = 0;
size_t fdLength = sizeof(fd);
if( ::zmq_getsockopt( _impl->socket, ZMQ_FD, &fd, &fdLength ) == -1 )
{
ZEQTHROW( std::runtime_error(
std::string( "Could not get socket descriptor'" )));
}
return fd;
}

bool Server::subscribe( servus::Serializable& object )
{
return _impl->subscribe( object );
Expand Down
11 changes: 11 additions & 0 deletions zeq/http/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ class Server : public zeq::Receiver
* adaptions. Also make zeq::URI( const servus::URI& from ) explicit.
*/
ZEQ_API const servus::URI& getURI() const;

/**
* Get the underlying socket descriptor.
*
* Can be used by client code to be notified when new data is available and
* subsequently call receive.
*
* @return the socket descriptor.
* @throw std::runtime_error if the descriptor could not be obtained.
*/
ZEQ_API SocketDescriptor getSocketDescriptor() const;
//@}

/** @name Object registration for PUT and GET requests */
Expand Down
6 changes: 6 additions & 0 deletions zeq/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ typedef std::shared_ptr< const uint8_t > ConstByteArray;
typedef std::vector< EventDescriptor > EventDescriptors;
typedef std::function< void( const Event& ) > EventFunc;

#ifdef WIN32
typedef SOCKET SocketDescriptor;
#else
typedef int SocketDescriptor;
#endif

/** Constant defining 'wait forever' in methods with wait parameters. */
// Attn: identical to Win32 INFINITE!
static const uint32_t TIMEOUT_INDEFINITE = 0xffffffffu;
Expand Down