Re: [async-mqtt5] Review questions

From: Ruben Perez (rubenperez038_at_[hidden])
Date: 2024-10-21 07:40:16

> >
> > You can now split the trait specialization to a separate, optional
> > websocket.hpp header, thus making Beast an optional peer dependency
> > (instead of a hard one). In this header, you can provide helper
> > typedefs to make things less verbose. E.g. using ws_client =
> > mqtt_client<boost::beast::websocket::stream<boost::asio::ip::tcp>>;
> >
> > If you combine this with my comment on placing TLS hooks in optional
> > headers, you'll end up with 4 optional headers, with a typedef each
> > (i.e. tcp_client, tcp_tls_client, ws_client, ws_tls_client). I think
> > this is great for usability.
> >
> >
>
> You're absolutely right that connect_op is too tightly coupled to the Beast WebSocket implementation. However, since it's the only available WebSocket library at the moment, it's difficult to define the correct type traits or specializations without having at least one other WebSocket implementation for reference. Currently, the if constexpr statements in connect_op explicitly instantiate Beast WebSocket objects and call their corresponding async methods. We'll need to decouple this, much in the way you've described.

I want to point out that I think that you can probably forward declare
the Beast websocket stream object in rebind_executor.hpp by replacing
the current rebind_executor implementation by:

template <typename Stream, bool deflate_supported, typename Executor>
struct rebind_executor<boost::beast::websocket::stream<asio::ssl::stream<Stream>,
deflate_supported>, Executor> {
    using other = typename boost::beast::websocket::stream<
        asio::ssl::stream<typename rebind_executor<Stream, Executor>::other>,
        deflate_supported
    >;
};

If you manage to do it, you can probably defer the introduction of
such trait classes until really required. IMO it is the hard
dependency what is a problem.

>
> Wow, thanks for these code snippets! If I'm not mistaken, you're suggesting creating an equivalent of asio::prepend/consign/append that operates directly on the CompletionHandler rather than the CompletionToken, which is exactly our main use case. This approach would allow us to avoid unnecessary token-to-handler specializations. Did I understand that correctly?

Yes. asio::prepend and asio::append also include code to deal with
arbitrary initiations, and my presumption is that a custom handler may
be cheaper. But you need to measure.

Note that what I wrote replace only append and prepend, and not
consign. For consign, you need to destroy the passed variables before
the handler is invoked. I can try to write something if you think it's
useful. Note that you can use asio::consign on top of the custom
handler that I wrote.

Regards,
Ruben.