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 camera support #336

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add camera support #336

wants to merge 6 commits into from

Conversation

anishsane
Copy link

It uses the mjpeg stream over the http server to access the camara feed.
The stream implementation is based on the mjpeg integration in home assistant.

It uses the mjpeg stream over the http server to access the camara feed.
This stream implementation is based on the mjpeg integration.
@anishsane
Copy link
Author

This needs an update in the tasmota integration of the home assistant to work end-to-end.

@emontnemery emontnemery changed the title Added camera support to hatasmota. Add camera support Jan 20, 2025
Copy link
Owner

@emontnemery emontnemery left a comment

Choose a reason for hiding this comment

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

Looks good, just questions about configurability

@emontnemery
Copy link
Owner

Please fix the lint errors 👍

@emontnemery
Copy link
Owner

There are still lint errors.
I'd recommend to install and run the linters locally, let me know if you need guidance for that.

I had defined them to support the flip/resolution/brightness change
operations.
However, it is easier to support these operations via
automation/blueprints, since these are just commands over mqtt.
Dropping them from the code to avoid the linter warnings.
@emontnemery
Copy link
Owner

emontnemery commented Feb 17, 2025

You need to add aiohttp to requirements.txt. We don't want to pin, so maybe set it to aiohttp>=3.0.0 (just the major version used by HA Core) or to aiohttp>=3.11.12 (the version currently used by HA Core)

@anishsane
Copy link
Author

My bad.
I tested in HA directly which already had it installed.
Fixed.

@anishsane
Copy link
Author

I had originally added the additional constants to support resolution/brightness/contrast configuration.
I was under the assumption that camera core integration has built-in support for such config knobs.

I implemented it via home assistant blueprints instead: tasmota/blueprints#4

@emontnemery
Copy link
Owner

I had originally added the additional constants to support resolution/brightness/contrast configuration. I was under the assumption that camera core integration has built-in support for such config knobs.

I implemented it via home assistant blueprints instead: tasmota/blueprints#4

Hmm, I'm not sure what you mean now. Can you elaborate a bit?

@emontnemery
Copy link
Owner

emontnemery commented Feb 21, 2025

Linters are still failing..

I'll explain how to set up the linters locally then.
First time, set up a local development environment for hatasmota:

cd hatasmota
python3 -m venv .venv
source .venv/bin/activate
pip3 install -r requirements.txt
pip3 install -r requirements-test.txt

After that, activate the local development environment for hatasmota:

cd hatasmota
source .venv/bin/activate

Run the linters:

flake8 hatasmota
pylint hatasmota
mypy hatasmota
black hatasmota
isort hatasmota --combine-as --force-sort-within-sections --profile black

Note that flake8 will complain about f-strings in existing code if run on Python 3.12 or newer, please ignore that

@emontnemery
Copy link
Owner

emontnemery commented Feb 21, 2025

The mypy error is genuine, websession.get returns an async context manager, not a StreamResponse.

I think it would make sense to modify the code like this:

diff --git a/hatasmota/camera.py b/hatasmota/camera.py
index 25fa14b..2cb4840 100644
--- a/hatasmota/camera.py
+++ b/hatasmota/camera.py
@@ -5,7 +5,7 @@ from __future__ import annotations
 from dataclasses import dataclass
 import logging
 from typing import Any
-from aiohttp import ClientSession, web
+from aiohttp import ClientResponse, ClientSession

 from .const import (
     CONF_IP,
@@ -122,12 +122,12 @@ class TasmotaCamera(TasmotaAvailability, TasmotaEntity):
         """Unsubscribe to all MQTT topics."""
         self._sub_state = await self._mqtt_client.unsubscribe(self._sub_state)

-    def get_still_image_stream(self, websession: ClientSession) -> web.StreamResponse | None:
+    async def get_still_image_stream(self, websession: ClientSession) -> ClientResponse:
         """Get the io stream to read the static image."""
         still_image_url = f"http://{self._cfg.ip_address}/snapshot.jpg"
-        return websession.get(still_image_url)
+        return await websession.get(still_image_url)

-    def get_mjpeg_stream(self, websession: ClientSession) -> web.StreamResponse | None:
+    async def get_mjpeg_stream(self, websession: ClientSession) -> ClientResponse:
         """Get the io stream to read the mjpeg stream."""
         mjpeg_url = f"http://{self._cfg.ip_address}:81/cam.mjpeg"
-        return websession.get(mjpeg_url)
+        return await websession.get(mjpeg_url)

You could also skip the await, and type the return values as Awaitable[ClientResponse]:

diff --git a/hatasmota/camera.py b/hatasmota/camera.py
index 25fa14b..a78a571 100644
--- a/hatasmota/camera.py
+++ b/hatasmota/camera.py
@@ -2,10 +2,11 @@

 from __future__ import annotations

+from collections.abc import Awaitable
 from dataclasses import dataclass
 import logging
 from typing import Any
-from aiohttp import ClientSession, web
+from aiohttp import ClientResponse, ClientSession

 from .const import (
     CONF_IP,
@@ -122,12 +123,12 @@ class TasmotaCamera(TasmotaAvailability, TasmotaEntity):
         """Unsubscribe to all MQTT topics."""
         self._sub_state = await self._mqtt_client.unsubscribe(self._sub_state)

-    def get_still_image_stream(self, websession: ClientSession) -> web.StreamResponse | None:
+    def get_still_image_stream(self, websession: ClientSession) -> Awaitable[ClientResponse]:
         """Get the io stream to read the static image."""
         still_image_url = f"http://{self._cfg.ip_address}/snapshot.jpg"
         return websession.get(still_image_url)

-    def get_mjpeg_stream(self, websession: ClientSession) -> web.StreamResponse | None:
+    def get_mjpeg_stream(self, websession: ClientSession) -> Awaitable[ClientResponse]:
         """Get the io stream to read the mjpeg stream."""
         mjpeg_url = f"http://{self._cfg.ip_address}:81/cam.mjpeg"
         return websession.get(mjpeg_url)

This commit also rearranges the code as per the linter suggestions.
@anishsane
Copy link
Author

Oh.. I had only used the pylint linter locally.
The other linters from your comment suggested changes.
Updated the PR.

About my blueprint-related comment:
My initial commit had some extra string constants added and imported. They were for configuring the camera settings like resolution. I had added them but I was not able to use them. I removed them from hatasmota and was able get that functionality using a home-assistant blueprint.

Comment on lines 398 to 412
def get_camera_entities(
discovery_msg: dict,
) -> list[tuple[TasmotaCameraConfig | None, DiscoveryHashType]]:
"""Generate camera configuration."""
camera_entities: list[tuple[TasmotaCameraConfig | None, DiscoveryHashType]] = []

entity = None
discovery_hash = (discovery_msg[CONF_MAC], "cam", "cam", 0)
if CONF_CAM in discovery_msg and discovery_msg[CONF_CAM]:
entity = TasmotaCameraConfig.from_discovery_message(discovery_msg, "camera")
camera_entities.append((entity, discovery_hash))

return camera_entities


Copy link
Owner

@emontnemery emontnemery Feb 21, 2025

Choose a reason for hiding this comment

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

Please move this to before get_cover_entities to keep the functions mostly sorted alphabetically

Copy link
Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 507 to 508
elif platform == "camera":
entities.extend(get_camera_entities(discovery_msg))
Copy link
Owner

Choose a reason for hiding this comment

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

Please move this before cover to keep the alphabetical sorting

Copy link
Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 537 to 538
if platform == "camera":
return TasmotaCamera(config=config, mqtt_client=mqtt_client)
Copy link
Owner

Choose a reason for hiding this comment

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

Please move this before cover to keep the alphabetical sorting

Copy link
Author

Choose a reason for hiding this comment

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

Done.

class TasmotaCameraConfig(TasmotaAvailabilityConfig, TasmotaEntityConfig):
"""Tasmota camera configuation."""

idx: int
Copy link
Owner

@emontnemery emontnemery Feb 21, 2025

Choose a reason for hiding this comment

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

Does tasmota support more than a single camera? If not, can we skip this, it seems to be hardcoded to 0 below?

Copy link
Author

Choose a reason for hiding this comment

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

Removed.


idx: int

command_topic: str
Copy link
Owner

Choose a reason for hiding this comment

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

This doesn't seem to be used

Copy link
Author

Choose a reason for hiding this comment

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

It was added earlier to support the configuring the camera parameters. But since it is not used, I have removed it.
The configuration is done easily via the blueprint mentioned earlier.

async def subscribe_topics(self) -> None:
"""Subscribe to topics."""

def state_message_received(msg: ReceiveMessage) -> None:
Copy link
Owner

Choose a reason for hiding this comment

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

What is this used for? The core PR doesn't seem to use it?

Copy link
Author

Choose a reason for hiding this comment

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

It was part of the original version of the core PR. It was removed during the review.
I have updated this PR to reflect that.

Earlier the sensors like fps and failures were used to set the camera
state. But this logic was removed from the tasmota component of the home-assistant.
Removing it from the hatasmota module as well.
Remove related now-unused imports from camera.py.

Minor changes:
It also rearranges the functions in discovery.py so that they are
alphabetical.
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