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.

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

Add websockets transport mechanism to MQTT #15601

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

VideoCurio
Copy link

Proposed change

Add websockets transport mechanism to MQTT.

An MQTT server is not always reachable only as raw TCP on port 1881; it can also be accessed as a WebSocket server, sometimes even behind a reverse proxy with TLS. Paho MQTT supports this feature, as does Mosquitto.

The default transport option was set to 'tcp', so this change will not affect an already configured Frigate app.

The configuration change is validated using a call to a pydantic library field validator.

This patch has been tested with Mosquitto 1.6 and 2.0, as well as with a server behind a reverse proxy on port 443 (requiring a TLS connection).

The docs/configuration/reference.md file has also been updated to reflect this change.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code
  • Documentation Update

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • The code has been formatted using Ruff (ruff format frigate)

## Proposed change
Add websockets transport mechanism to MQTT.

An MQTT server is not always reachable only as raw TCP on port 1881; it can also be accessed as a WebSocket server, sometimes even behind a reverse proxy with TLS. Paho MQTT supports this feature, as does Mosquitto.

The default transport option was set to 'tcp', so this change will not affect an already configured Frigate app.

The configuration change is validated using a call to a pydantic library field validator.

This patch has been tested with Mosquitto 1.6 and 2.0, as well as with a server behind a reverse proxy on port 443 (requiring a TLS connection).

The docs/configuration/reference.md file has also been updated to reflect this change.

## Type of change

- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New feature
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code
- [x] Documentation Update

## Additional information

- This PR fixes or closes issue: fixes blakeblackshear#15600
- This PR is related to issue:

## Checklist

- [x] The code change is tested and works locally.
- [ ] Local tests pass. **Your PR cannot be merged unless tests pass**
- [x] There is no commented out code in this PR.
- [x] The code has been formatted using Ruff (`ruff format frigate`)
Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for frigate-docs ready!

Name Link
🔨 Latest commit 56a128b
🔍 Latest deploy log https://app.netlify.com/sites/frigate-docs/deploys/67644ab729f2ea00084afde4
😎 Deploy Preview https://deploy-preview-15601--frigate-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -15,6 +15,7 @@ class MqttConfig(FrigateBaseModel):
enabled: bool = Field(default=True, title="Enable MQTT Communication.")
host: str = Field(default="", title="MQTT Host")
port: int = Field(default=1883, title="MQTT Port")
transport: str = Field(default="tcp", title="MQTT Transport Mechanism")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be better if we added an enum class to MQTT and used that vs a manual field validator.

@@ -168,9 +168,11 @@ def _on_disconnect(

def _start(self) -> None:
"""Start mqtt client."""
logger.info("MQTT transport mechanism: %s" % str(self.mqtt_config.transport))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this should be an info log.

Copy link
Author

Choose a reason for hiding this comment

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

ok, debug maybe or get rid of this line ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Debug seems fine

@hawkeye217
Copy link
Collaborator

You'll also need to ensure your code is formatted using ruff as the linter is failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add websockets transport mechanism to MQTT
3 participants