Skip to content

v1.4.0-beta2#64

Open
slaxor505 wants to merge 10 commits intoOlen:mainfrom
slaxor505:main
Open

v1.4.0-beta2#64
slaxor505 wants to merge 10 commits intoOlen:mainfrom
slaxor505:main

Conversation

@slaxor505
Copy link
Contributor

The latest changes from v1.3.3-beta1 have been merged in this PR.

New features and updates:

• Updated openplantbook-sdk to v0.6.1, adding language configuration for plant data fetches, improved authentication error handling, and minor text enhancements.
• Added image download service tests and merged image download improvements.
• Introduced plant sensor monitoring with interactive UI notifications and improved image download error handling.
• Added upload schedule randomizer.
• Added rate limit error handling across services and upload flow, plus UI warnings and one-off feature notification updates.
• Merged post-merge updates: new tests, documentation updates, message refinements, and strings.json/en.json corrections.
• Version bumps to 1.4.0-beta1 and formatting/ruff fixes.

- Added support for language configuration when fetching plant data
- Improved error handling for authentication failures
- Minor text updates and enhancements

Took 7 hours 26 minutes
…ications

Improved error handling when downloading images

Took 10 hours 18 minutes
…emented UI warnings for monitoring and updated one-off feature notification.

Took 2 hours 10 minutes
# Conflicts:
#	custom_components/openplantbook/__init__.py
#	custom_components/openplantbook/config_flow.py
#	custom_components/openplantbook/manifest.json
#	custom_components/openplantbook/strings.json
#	custom_components/openplantbook/translations/en.json
#	custom_components/openplantbook/uploader.py
#	info.md
#	pytest.ini
#	tests/test_uploader.py

Took 12 minutes
Updated documentation
Refining messages text
Corrected strings and en json after merge
Versions to 1.4-beta1
Black formatted

Took 2 hours 43 minutes
Took 10 minutes
Took 37 minutes
@Olen
Copy link
Owner

Olen commented Feb 21, 2026

Thanks for all the work on this, @slaxor505! Really appreciate you maintaining the OpenPlantbook side and pushing the integration forward. This is a substantial PR with a lot of good stuff in it.

What I like

  • Rate limit handling — catching RateLimitError across all services and surfacing it properly via HomeAssistantError is exactly right. Much better UX than silent failures.
  • Language support — sending HA's language to the API for internationalized common names is a great feature. Clean implementation with the FLOW_SEND_LANG option.
  • Permission error handling — both the auth failure path and the graceful image-write fallback are solid improvements.
  • Upload schedule randomizerrandom.Random(entry.entry_id) for a stable random daily time is clever and avoids thundering-herd on the API.
  • New tests — the rate limit, permission error, and uploader tests are thorough and cover real scenarios.
  • SDK bump to 0.6.1 — good to stay current.

Adding you as codeowner is fine, and I think this change justifies moving to 1.4.0.

Suggested improvements

Must fix

  1. .junie/guidelines.md — This is a JetBrains Junie AI agent config file. Should be removed from the PR (and maybe added to .gitignore).

  2. CannotConnect class is dead code — defined at the bottom of __init__.py but never used anywhere. Should be removed.

  3. pytestmark = pytest.mark.enable_socket in test_service_permission_error.py and test_service_rate_limit_error.py — this mark isn't registered in the project's pytest config and generates warnings. The tests mock everything, so they don't need socket access. The marker can just be removed.

Sensor monitoring — does it belong here?

I'm not sure the persistent notification system for stale/missing sensors belongs in the openplantbook integration. Logging warnings during uploads about stale data makes perfect sense here — it helps debug why uploads are empty. But user-facing persistent notifications about sensor health feel like they belong in the plant integration, which already does problem detection (moisture low, temperature out of range, etc.). Stale-sensor detection would be a natural fit there.

The "no recent upload" and "never uploaded" warnings are purely OPB upload concerns and make sense here. But the per-sensor staleness notifications are really about plant health monitoring.

What do you think? Could we keep the log-level warnings during upload but drop the persistent notification part (or move it to the plant integration)?

Code quality suggestions

  1. f-string logging — Throughout uploader.py there are patterns like _LOGGER.debug(f"...") instead of lazy _LOGGER.debug("...", arg). With f-strings the string formatting happens even when the log level is disabled, which is wasteful during normal operation.

  2. Test boilerplatetest_uploader.py is 1200+ lines with a lot of repeated mock setup (device registry, entity registry, recorder mocks). Shared fixtures in conftest.py could cut this down significantly and make the tests easier to maintain.

  3. PLANTBOOK_BASEURL in SDK constructor — The SDK already defaults to this URL, so passing base_url=PLANTBOOK_BASEURL explicitly creates a maintenance point where the constant and the SDK default could drift apart.

Minor

  1. Notification text links to https://github.com/slaxor505/OpenPlantbook-client/wiki/... — should this point to the main project's docs instead?

  2. OPB_CURRENT_INFO_MESSAGE bumped to 2 — this means every existing user gets a new popup. The old code skipped the notification if upload was already enabled; the new code always shows it. Worth considering whether that's the right UX.


Note: I've already resolved the merge conflicts with PR #65 (DLI conversion) locally — the conflicts were in const.py and __init__.py imports. Both feature sets coexist cleanly and all 90 tests pass.

@slaxor505
Copy link
Contributor Author

Hi @Olen,

Thanks for the quick response!

Sure I can fix minor things.

  1. In answer to your questions "Sensor monitoring — does it belong here?", sensor monitoring is and always was part of uploading functionality and cannot function without it. Stale state warnings were implemented back in 1.3. In this 1.4 version I refined the monitoring and added optional interactive UI notification for user's convenience. I'm happy to hear your suggestions on how to make it part of Plant integration, but at this stage my opinion is that this is right place for it. The notification is optional so users can disable it if necessary and when similar functionality available in Plant integration.

  2. PLANTBOOK_BASEURL constant here for testing and development purpose as there is no easy way to re point integration to the different URL.

  3. https://github.com/slaxor505/OpenPlantbook-client/wiki/ this link is to OPB documentation and it is not related to the integration.

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.

2 participants