Skip to content

Add Govee Enclosure plugin - #1479

Open
RobertAWells wants to merge 2 commits into
OctoPrint:gh-pagesfrom
RobertAWells:gh-pages
Open

Add Govee Enclosure plugin#1479
RobertAWells wants to merge 2 commits into
OctoPrint:gh-pagesfrom
RobertAWells:gh-pages

Conversation

@RobertAWells

Copy link
Copy Markdown
  • You have read the "Registering a new Plugin" guide.
  • You want to and are able to maintain the plugin you are registering, long-term.
  • You understand why the plugin you are registering works.
  • You have read and acknowledge the Code of Conduct.

What is the name of your plugin?

Govee Enclosure

What does your plugin do?

Govee Enclosure monitors Govee H5179/GV5179 temperature and humidity sensors locally over Bluetooth LE. It displays enclosure temperature, humidity, and battery status in the OctoPrint sidebar and adds enclosure temperature to OctoPrint's native Temperature graph. The sidebar temperature display can be configured for Celsius or Fahrenheit.

Where can we find the source code of your plugin?

https://github.com/RobertAWells/OctoPrint-GoveeEnclosure

Was any kind of genAI (ChatGPT, Copilot etc) involved in creating this plugin?

Yes. I used ChatGPT to assist with code development, debugging, documentation, and packaging. I tested the plugin on my OctoPrint systems and reviewed how the plugin works.

Is your plugin commercial in nature?

No. It is open-source software released under the MIT license.

Does your plugin rely on some cloud services?

No. Sensor readings are obtained locally over Bluetooth LE and do not require the Govee cloud service.

Tested with Govee H5179/GV5179 sensors on Raspberry Pi-based OctoPrint installations. A working Bluetooth adapter is required.

Further notes

Add Govee Enclosure plugin documentation for monitoring temperature and humidity via Bluetooth LE.

compatibility:
octoprint:
- ">=1.11,<2"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Giving that we are going to release OctoPrint 2.0.0 soon (currently is under RC), I believe you should support that one too

@jneilliii

Copy link
Copy Markdown
Contributor

I agree with @jacopotediosi about the OctoPrint 2.0.0 support. Better to go ahead and validate now against that version and support it since we'll be releasing right around the corner. Only thing that octoscanner found requires a minor update to your code.

| | +---------------------------------------- Summary (1 total findings) -----------------------------------------+ | |
| | | 1 security                                                                                                  | | |
| | +-------------------------------------------------------------------------------------------------------------+ | |
| | +------------------------------------------- Security (1 findings) -------------------------------------------+ | |
| | | SEC-0007                                                                                                    | | |
| | | Plugin implements TemplatePlugin but does not override is_template_autoescaped(). Until OctoPrint 2.1.0,    | | |
| | | the default implementation returns False, exposing the templates to XSS via unescaped variables.            | | |
| | | File: octoprint_govee_enclosure\__init__.py:18                                                              | | |
| | |       17 |                                                                                                  | | |
| | |   >   18 | class GoveeEnclosurePlugin(                                                                      | | |
| | |       19 |     octoprint.plugin.SettingsPlugin,                                                             | | |
| | | Suggestion: Override is_template_autoescaped() to return True and use the safe Jinja filter only for        | | |
| | | variables that legitimately need to render HTML. See https://faq.octoprint.org/plugin-autoescape.           | | |
| | +-------------------------------------------------------------------------------------------------------------+ | |

I don't see any major hallucinations in this one, except when it comes to the update hook. I don't think you need these two lines at all.

https://github.com/RobertAWells/OctoPrint-GoveeEnclosure/blob/a15fc7d9c68a438596709d56d1e276a1bcb52df5/octoprint_govee_enclosure/__init__.py#L134-L135

and I don't know why you would parameterize your github user and repo name to fill in the update hook data, etc., just seems odd to not hard code those directly unless you want to make it easier for someone to adopt the plugin in the future when you decide to not maintain the plugin anymore.

I do appreciate the extraction of the packet decoding from govee-ble in order to continue support of python 3.9, which will align well with OctoPrint 2.0.0 as python 3.9 becomes it's lowest supported version.

@jneilliii

Copy link
Copy Markdown
Contributor

One thing that could be done differently would be to not monkey patch the temperature graph in the js file, but use the chamber temperature option in the printer profile and return the parsed_temps with the "C" property, similar to the plugin below:

https://github.com/synman/OctoPrint-MqttChamberTemperature/blob/c303007827ce966072cb010da36e6a402abc79ff/octoprint_mqttchambertemperature/__init__.py#L241-L248

@jacopotediosi jacopotediosi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks so much for your contribution, @RobertAWells 😄

I finally got around to giving this a proper review.

I'm on board with everything discussed above - in particular, I agree with @jneilliii that you shouldn't monkey-patch OctoPrint's temperature graph.

One last thing I spotted: in __init__.py you're overriding a bunch of plugin properties that don't really need to, e.g. __plugin_version__, __plugin_name__, etc. There's a risk they'll drift out of sync with the ones already defined in pyproject.toml, thus we generally recommend removing them unless strictly necessary.

Once those are sorted, this plugin is good to go from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants