chirpstack / chirpstack-gateway-bridge

ChirpStack Gateway Bridge abstracts Packet Forwarder protocols into Protobuf or JSON over MQTT.
https://www.chirpstack.io
MIT License
423 stars 270 forks source link

Make `maximum seconds that will be waited between reconnection attempts when connection is lost` to be configurable #84

Closed moznion closed 6 years ago

moznion commented 6 years ago

Now the default interval is 10m that is configured at https://github.com/eclipse/paho.mqtt.golang/blob/9ec68b7894ac4cd48fc9126e910e571d50eb5d9c/options.go#L102 This pull request makes the interval to be configurable.

brocaar commented 6 years ago

Thanks! Would it be possible to change the config option to max_reconnect_interval and format it as a string? E.g.

# Maximum seconds that will be waited between reconnection attempts when connection is lost.
# Valid units are 'ms', 's', 'm', 'h'. Note that these values can be combined, e.g. '24h30m15s'.
max_reconnect_interval_seconds=10m

This is more consistent with other config options that define an interval, e.g. (from loraserver.toml):

# Device session expiration.
#
# The TTL value defines the time after which a device-session expires
# after no activity. Valid units are 'ms', 's', 'm', 'h'. Note that these
# values can be combined, e.g. '24h30m15s'.
device_session_ttl="744h0m0s"
moznion commented 6 years ago

Sorry for late reaction. A pointed topic at the above is solved by https://github.com/moznion/lora-gateway-bridge/commit/293348a8dc485b5e33a6f17707b929df8e124950 💪

brocaar commented 6 years ago

Sorry for late reaction. A pointed topic at the above is solved by soracom/lora-gateway-bridge@293348a 💪

That link doesn't work for me? I've added some in-line comments. Could you make these changes? Alternatively, I can also pull your changes make the modifications and push these. Let me know what you prefer.

moznion commented 6 years ago

@brocaar I'm so sorry, I posted wrong URL. Actually https://github.com/moznion/lora-gateway-bridge/commit/293348a8dc485b5e33a6f17707b929df8e124950 is correct, and I pushed that.

brocaar commented 6 years ago

Why did you change the MaxReconnectInterval from time.Duration to string? When you use time.Duration and set the default to 10 * time.Minute you don't have to deal with the string -> time.Duration parsing yourself as this is done automatically for you :-) Could you change this back?

moznion commented 6 years ago

I didn't know that the time.Duration style text option is converted to time.Duration automatically.

Thank you for pointing it out. I fixed that at 192e164.

brocaar commented 6 years ago

Thanks @moznion 👍

moznion commented 6 years ago

I appreciate your help.