eclipse-paho / paho.mqtt.golang

Other
2.77k stars 534 forks source link

add cuscom dialer #569

Closed carr123 closed 2 years ago

carr123 commented 2 years ago

hi, i add cuscom dialer to support more underlying transport types.

it's easy to use.

demo :

import ( mqtt "github.com/eclipse/paho.mqtt.golang" )

func main() { mqtt.AddCustomDialer( "haha", func(uri url.URL, tlsc tls.Config, timeout time.Duration, headers http.Header) (net.Conn, error) { return HahaConnect(uri.Host, tlsc, timeout) }, )

  BrokerURL := "haha://127.0.0.1:5883"

}

MattBrittan commented 2 years ago

Thanks for the PR. Unfortunately I cannot progress this until you have signed the ECA (see the commit checks and readme for details).

However I have taken a quick look and am going to suggest a different approach (happy to discuss this!). Please don't take this the wrong way, your approach is fine and would work, but I think the below may be a better fit.

My aims with this suggestion are:

I'm also going to suggest that we allow openConnection to be replaced in its entirety rather than just adding in new URL handler types.

So lets start with a ClientOption:

type openConnectionFunc func(uri *url.URL, tlsc *tls.Config, timeout time.Duration, headers http.Header, websocketOptions *WebsocketOptions) (net.Conn, error)
.
.
.
func NewClientOptions() *ClientOptions {
openConnectionFunc: openConnection // default to inbuilt function
.
.
.

// SetOpenConnectionFunc replaces the inbuilt function that establishes a connection with a custom function.
// The passed in function should return an open `net.Conn` or an error (see the existing openConnection function for an example)
func (o *ClientOptions) SetOpenConnectionFunc(fn openConnectionFunc) *ClientOptions {
    o.openConnectionFunc = fn
    return o
}

and then we can replace the call to
conn, err = openConnection(broker, tlsCfg, c.options.ConnectTimeout, c.options.HTTPHeaders, c.options.WebsocketOptions) in client.go with:

c.Options.openConnectionFunc(broker, tlsCfg, c.options.ConnectTimeout, c.options.HTTPHeaders, c.options.WebsocketOptions)

I think that this would meet your needs along with some other potential future requirements whilst limiting the level of change. Thoughts?

carr123 commented 2 years ago

i agree with you. your way is much better. i will check what is ECA ,and summit a new PR. :)

MattBrittan commented 2 years ago

OK thanks - I'll close this pull request.