search

home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.19k stars 30.57k forks source link

Amcrest Camera error: Bad request for VideoMotion #33875

Closed GeorgeSG closed 4 years ago

GeorgeSG commented 4 years ago

The problem

After I upgraded from Home Assistant Core 0.107.7 to 0.108.1, I see the following warnings and errors from my Amcrest Camera:

It retries 30 times:

WARNING (Amcrest Amcrest Camera) [amcrest.http] <Camera Name> Trying again due to error: HTTPError('400 Client Error: Bad Request for url: http://camera-ip:80/cgi-bin/eventManager.cgi?action=attach&codes=%5BVideoMotion%5D')

Followed by:

ERROR (Amcrest Amcrest Camera) [homeassistant.components.amcrest] Amcrest Camera camera offline: Too many errors

This repeats every couple of seconds.

The camera model is IP2M-841-V3

Removing the motion_detected binary sensor from the config fixes the issue.

Environment

Problem-relevant configuration.yaml

amcrest:
  - host: !secret amcrest_ip
    username: !secret amcrest_username
    password: !secret amcrest_password
    resolution: low
    binary_sensors:
      - motion_detected
      - online

Traceback/Error logs

WARNING (Amcrest Amcrest Camera) [amcrest.http] <Camera Name> Trying again due to error: HTTPError('400 Client Error: Bad Request for url: http://camera-ip:80/cgi-bin/eventManager.cgi?action=attach&codes=%5BVideoMotion%5D')

ERROR (Amcrest Amcrest Camera) [homeassistant.components.amcrest] Amcrest Camera camera offline: Too many errors

Additional information

I think this may have been caused by #32818, but I don't have a good understanding of the Amcrest API, so I may be wrong.

Motion detection used to work in 0.107.7.

probot-home-assistant[bot] commented 4 years ago

Hey there @pnbruckner, mind taking a look at this issue as its been labeled with a integration (amcrest) you are listed as a codeowner for? Thanks!

pnbruckner commented 4 years ago

@GeorgeSG please see: https://community.home-assistant.io/t/amcrest-testers-wanted-more-responsive-motion-detection/165768

Unfortunately there are many Amcrest camera models and many different firmware versions, so it's really hard to test that anything will work correctly with all of them. I tried to get feedback from users but didn't get much. But what I did get seemed to indicate this would work. It certainly does with many.

Also unfortunate is that Amcrest has released some new models, and new firmware for older models, that doesn't comply with their own HTTP API spec. I've reached out to Amcrest multiple times in multiple ways but they won't provide an updated spec. In fact, the won't even confirm that anything has changed, which it clearly has.

So at this point I'm not sure what to tell you. You could try downgrading your camera firmware. (This isn't the only thing that has issues with newer firmware.)

Also, if you could provide some feedback based on the forum topic for which I provided a link above, that would be helpful.

jamesarbrown commented 4 years ago

I am also experiencing this, but I dont think its due to firmware, my Dahua IPC are somewhat old and the firmware latest and last is 2017 from memory.

If I drop the error into a browser http://192.168.1.41:80/cgi-bin/eventManager.cgi?action=attach&codes=%5BVideoMotion%5D

I get this (tab immediately stops) image

But then click refresh and (tab is continually connecting) image

But no response.. not sure how this is meant to work, maybe it just sits alive waiting for an event? So maybe in daylight I need to try and create a motion and see if the page updates "live"

jamesarbrown commented 4 years ago

As an update, this does appear to be working via browser as I got a response like this... I will look again this evening.

-- myboundary Content-Type: text/plain Content-Length:36 Code=VideoMotion;action=Stop;index=0

pnbruckner commented 4 years ago

@jamesarbrown that looks like the right output. What are you seeing in the HA logs? Is there anyway you can run the test Python script (independently of HA) that you can find via the forum link I provided above?

jamesarbrown commented 4 years ago

As per HA, it does not work. But if I drop a non-URLEncoded url in firefox it works (200). If I drop a URLEncoded (%5b/d) in firefox it gives a 400.

I also tried with "All" in place of VideoMotion

So the only things I can see different when using browser, when it works it report HTTP1.0 and is non URL encoded in browser. When it does not work its HTTP1.1 and URL encoded string.

Is there anyway to run this script without having python http encoding it?

I could make this camera internet available if that is of any use.

Error

`bash-5.0# python3 test_events.py 192.168.1.41 80 admin somepass

Trying again due to error: HTTPError('400 Client Error: Bad Request for url: http://192.168.1.41:80/cgi-bin/eventManager.cgi?action=attach&codes=%5BAll%5D') Traceback (most recent call last): File "/usr/local/lib/python3.7/site-packages/amcrest/http.py", line 156, in _command resp.raise_for_status() File "/usr/local/lib/python3.7/site-packages/requests/models.py", line 940, in raise_for_status raise HTTPError(http_error_msg, response=self) requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: http://192.168.1.41:80/cgi-bin/eventManager.cgi?action=attach&codes=%5BAll%5D During handling of the above exception, another exception occurred: Traceback (most recent call last): File "test_events.py", line 48, in main() File "test_events.py", line 30, in main timeout_cmd=(3.05, None), stream=True) File "/usr/local/lib/python3.7/site-packages/amcrest/http.py", line 138, in command return self._command(cmd, retries, timeout_cmd, stream) File "/usr/local/lib/python3.7/site-packages/amcrest/http.py", line 162, in _command raise CommError(error) amcrest.exceptions.CommError: 400 Client Error: Bad Request for url: http://192.168.1.41:80/cgi-bin/eventManager.cgi?action=attach&codes=%5BAll%5D `
mkbrown commented 4 years ago

I have the same issue upgrading from 0.107.7 to 0.108.6, with the 400 Client Errors on generic Dahau cameras. They don’t like the command URLencoding the []. I did a quick and dirty patch to event.py, and got it working...


--- event.py.orig       2020-04-17 22:16:11.488167900 -0400
+++ event.py    2020-04-17 23:21:46.830314848 -0400
@@ -203,7 +203,8 @@
                 timeout_cmd = (self._timeout_default, None)

         ret = self.command(
-            "eventManager.cgi?action=attach&codes=[{0}]".format(eventcodes),
+            'eventManager.cgi?action=attach&codes=',
+            param='[All]',
             retries=retries,
             timeout_cmd=timeout_cmd,
             stream=True,

The param appends without URLencoding. Hope that helps!

/Mike

GeorgeSG commented 4 years ago

Hey all,

I was experiencing the exact same issue with URL encoding. I was running a very old firmware - August 2016, so I decided to pull the trigger and upgrade it to the latest one available for my camera (February 2020). The issue is apparently fixed and the motion sensor works.

Here's the output from test_events.py after the firmware upgrade:

2020-04-18 09:49:28 '\r\nCode=VideoMotion;action=Start;index=0;data=nul'
2020-04-18 09:49:50 '\r\nCode=VideoMotion;action=Stop;index=0;data=nul'
2020-04-18 09:49:58 '\r\nCode=VideoMotion;action=Start;index=0;data=nul'
2020-04-18 09:50:10 '\r\nCode=VideoMotion;action=Stop;index=0;data=nul'
2020-04-18 09:50:16 '\r\nCode=VideoMotion;action=Start;index=0;data=nul'
jamesarbrown commented 4 years ago

Hi @GeorgeSG, the problem I have and I am sure many others, is that Dahua only support firmwares for a short while. For example this is the last firmware for mine... as far as i know Device Type IPC-HDW4300S Software Version 2.420.0009.0.R, build : 2015-11-06 WEB Version 3.2.1.305503 ONVIF Version 2.4.1

So given that the cameras clearly have a bug for not being able to accept a proper encoded URL, ill flag a bug on Python Amcrest and see if any miles are achieved.

GeorgeSG commented 4 years ago

@jamesarbrown, yeah, I get that. I'm not familiar with Dahua and I also saw @pnbruckner's warning that newer Amcrest firmware may cause other problems.

So, yeah, IMO this should be patched. I just wanted to document my "fix" in case someone else with the same camera model stumbles upon this :)

jamesarbrown commented 4 years ago

@mkbrown - That seems to work for me. Thanks. @GeorgeSG - As far as I know Amcrest = Branded Dahua for US

@pnbruckner I have logged an upstream bug here https://github.com/tchellomello/python-amcrest/issues/155

Mkbrown dirty patch fixed HA for me. Could your test_events.py be also adjusted to remove http encoding or use the amcrest py rather than http directly?

pnbruckner commented 4 years ago

@GeorgeSG @jamesarbrown @mkbrown

Thanks for all the details.

FWIW, the amcrest package just uses ~requests.Session().get and passes it the un-encoded URL. (You can see this if you enable debug for amcrest. It will output the URL used.) If the URL is getting encoded, that's happening within the requests package.

Regarding changing the request from ...codes=[VideoMotion] to ...codes=[All], I'm not sure how that changes how the URL is encoded (or not.)

Regarding test_events.py, that does use the amcrest package, not requests or anything else directly, although it uses the "lower level" command method. But that's effectively the same as HA, since it eventually gets down to using the same command method.

pnbruckner commented 4 years ago

So I did a quick look around requests, urllib3, etc., and again, I don't see how changing from [VideoMotion] to [All] changes the way the URL is encoded. In fact:

>>> from urllib.parse import urlencode
>>> urlencode({'code': '[VideoMotion]'})
'code=%5BVideoMotion%5D'
>>> urlencode({'code': '[All]'})
'code=%5BAll%5D'

Maybe the problem is some of the Dahua firmware versions don't accept VideoMotion as a code, but do accept All.

pnbruckner commented 4 years ago

I just checked in a new version of test_events.py that will enable debug and output the log as it runs. Just append log to the end of the command line when invoking the script.

mkbrown commented 4 years ago

The issue with our camera’s firmware is the encoding of the Brackets themselves; the %5B vs the [ in the URL. If it’s a bracket in the URL, it works fine. If it’s encoded as %5B and %5D, we get HTTP 400.

pnbruckner commented 4 years ago

@mkbrown I understand. But as I said, that encoding is happening within the requests package, probably because that is what is required. And as I just saw in your response in the amcrest package issue your code modification didn't work as you expected. It effectively broke the command instead of fixing it.

The bottom line seems to be that some older Dahua camera firmware doesn't handle the URLs correctly, which was apparently fixed at some point. I'm not sure working around a bug like this is the right thing to do. However, if a valid way can be shown how to get this to work for those cameras, without breaking it for others, then of course I'm willing to try it.

jamesarbrown commented 4 years ago

Rather than a workaround can't HA retain the older polling method within the code and have two sensor options?

MotionDetect (EventTrigger) & MotionDetectPoll - Supporting older cameras

I appreciate working round a bug within the camera seems a bad ethics, but there are a lot of cameras out there that will be carrying this. I have a newer IPC-HW4431 on the way, will be interesting to see.

jamesarbrown commented 4 years ago

So I did a quick look around requests, urllib3, etc., and again, I don't see how changing from [VideoMotion] to [All] changes the way the URL is encoded. In fact:

>>> from urllib.parse import urlencode
>>> urlencode({'code': '[VideoMotion]'})
'code=%5BVideoMotion%5D'
>>> urlencode({'code': '[All]'})
'code=%5BAll%5D'

Maybe the problem is some of the Dahua firmware versions don't accept VideoMotion as a code, but do accept All.

My browser supported both VideoMotion and All events with [] as long as we do not URLencode %5b etc. With url encoding the events, All, VideoMotion do not work.

mkbrown fix cleared the HA fault, so assumed HA was getting through, but then I also did not have time to see if the trigger worked..... and my suspicion was that it wasnt working.

pnbruckner commented 4 years ago

@jamesarbrown if you'd like to try adding a polling option, you could do it by modifying homeassistant/components/amcrest/binary_sensor.py. Find this code starting on line 32:

BINARY_SENSOR_MOTION_DETECTED = "motion_detected"
BINARY_SENSOR_ONLINE = "online"
BINARY_SENSORS = {
    BINARY_SENSOR_MOTION_DETECTED: (
        "Motion Detected",
        DEVICE_CLASS_MOTION,
        "VideoMotion",
    ),
    BINARY_SENSOR_ONLINE: ("Online", DEVICE_CLASS_CONNECTIVITY, None),
}

Change it to:

BINARY_SENSOR_MOTION_DETECTED = "motion_detected"
BINARY_SENSOR_MOTION_DETECTED_POLLED = "motion_detected_polled"
BINARY_SENSOR_ONLINE = "online"
BINARY_SENSORS = {
    BINARY_SENSOR_MOTION_DETECTED: (
        "Motion Detected",
        DEVICE_CLASS_MOTION,
        "VideoMotion",
    ),
    BINARY_SENSOR_MOTION_DETECTED_POLLED: (
        "Motion Detected Polled",
        DEVICE_CLASS_MOTION,
        None,
    ),
    BINARY_SENSOR_ONLINE: ("Online", DEVICE_CLASS_CONNECTIVITY, None),
}

And this code:

    @property
    def should_poll(self):
        """Return True if entity has to be polled for state."""
        return self._sensor_type == BINARY_SENSOR_ONLINE

Change to:

    @property
    def should_poll(self):
        """Return True if entity has to be polled for state."""
        return self._event_code is None

Then change your amcrest config from using motion_detected to motion_detected_polled and restart HA. Let me know how that works for you.

jamesarbrown commented 4 years ago

No i am sorry, it did not work, it certainly comes up in HA showing clear, but no events picked up.

I tested like this to prove camera

bash-5.0# amcrest-cli --camera cam1 --event-channels-happened VideoMotion
Error: No Events

and when it gets an event

bash-5.0# amcrest-cli --camera cam1 --event-channels-happened VideoMotion
channels[0]=0

So I guess the camera is re-proven

Cant really see a way forward here, might try to crash some newer firmware on there, i even found http://192.168.1.41:80/cgi-bin/IntervideoManager.cgi?action=getVersion&Name=CGI throws a 500 code

pnbruckner commented 4 years ago

@jamesarbrown did you enable debug to see if there were any debug messages in home-assistant.log that might shed some light on what's happening? E.g.:

logger:
  default: error
  logs:
    amcrest: debug
    homeassistant.components.amcrest: debug
jamesarbrown commented 4 years ago

I think the issue is, the poll interval is 60sec. I will take a look later and see if scan_interval is settable, but something in my mind, it used to be 10sec by default.

But yes the camera is being polled 2020-04-21 07:42:40 DEBUG (SyncWorker_4) [homeassistant.components.amcrest.binary_sensor] Updating Amcrest192.168.1.41 Motion Detected Polled binary sensor 2020-04-21 07:42:40 DEBUG (SyncWorker_4) [amcrest.http] <TZC4KX495W00041:TZC4KX495W00041> HTTP query 20: http://192.168.1.41:80/cgi-bin/eventManager.cgi?action=getEventIndexes&code=None 2020-04-21 07:42:40 DEBUG (SyncWorker_4) [amcrest.http] <TZC4KX495W00041:TZC4KX495W00041> Running query 20 attempt 1 2020-04-21 07:42:40 DEBUG (SyncWorker_4) [amcrest.http] <TZC4KX495W00041:TZC4KX495W00041> Query 20 worked. Exit code: <200> 2020-04-21 07:42:40 DEBUG (SyncWorker_12) [homeassistant.components.amcrest.binary_sensor] Updating Amcrest192.168.1.41 Online binary sensor 2020-04-21 07:42:40 DEBUG (SyncWorker_12) [amcrest.http] <TZC4KX495W00041:TZC4KX495W00041> HTTP query 21: http://192.168.1.41:80/cgi-bin/global.cgi?action=getCurrentTime 2020-04-21 07:42:40 DEBUG (SyncWorker_12) [amcrest.http] <TZC4KX495W00041:TZC4KX495W00041> Running query 21 attempt 1 2020-04-21 07:42:40 DEBUG (SyncWorker_12) [amcrest.http] <TZC4KX495W00041:TZC4KX495W00041> Query 21 worked. Exit code: <200>

pnbruckner commented 4 years ago

Ah, yes, I forgot about the polling interval. It used to be 5 seconds for motion detection and longer for the online sensor. Assuming I officially add a polling version for motion detection I'll have to add back in the code for controlling the polling interval as before.

jamesarbrown commented 4 years ago

Hi @pnbruckner, so am I right thinking these are the choices now ?

Most appreciative of your support to work through, but I guess you have a decision to make :)

As mentioned I have a much new camera on the way as recently moved house, so quite willing to try new and old cameras.

pnbruckner commented 4 years ago

I considered trying to implement some sort of failure detection & fallback mechanism, but the problem is, even when the camera supports the correct way/URL, it sometimes still fails due to network issues, etc. So it's not very straightforward to know when to switch to a fallback mechanism, whether that be forcing an incorrect URL or switching from event streaming to polling. And it's further complicated by all the permutations of camera model & firmware version, and no real way to test them all before submitting a change.

So, bottom line, I have no plans to attempt to implement any workarounds for some cameras that do not properly handle correct URLs (as I understand it.)

I'm considering adding back polling mode for motion detection as an alternative option (i.e., pick event stream or polled mode), but that is more complicated than it appears on the surface, especially since the same would have to be done for any other event types that might be added later (one of which may already be in the works.) Unfortunately I haven't been able to work on HA stuff as much recently as I used to, so I can't make any commitments as to when I might do this.

jamesarbrown commented 4 years ago

I took a 3rd look at the firmware for reference it is this firmware, other firmwares may give differing mileage. [PAL]DH_IPC-HX5(4)XXX-Adreia_Eng_P_Stream3_V2.420.0009.0.R.20151106

Affecting these cameras DH-IPC-HFW5502C,DH-IPC-HDBW5502,DH-IPC-HDB5502,DH-IPC-HFW5300E-VF,DH-IPC-HFW5300E-Z,DH-IPC-HFW5300C,DH-IPC-HFW5302C,DH-IPC-HFW5300C-L,DH-IPC-HDBW5300,DH-IPC-HDB5300,DH-IPC-HDBW5302,DH-IPC-HDB5302,DH-IPC-HFW4300E,DH-IPC-HFW4300S,DH-IPC-HDBW4300E,DH-IPC-HDW4300S,DH-IPC-HDW4300C,DH-IPC-HDB4300C,DH-IPC-HDB4300F-PT,DH-IPC-HF5200,DH-IPC-HFW5200E-VF,DH-IPC-HFW5200E-Z,DH-IPC-HFW5200C,DH-IPC-HFW5202C,DH-IPC-HFW5200C-L,DH-IPC-HFW5200-IRA,DH-IPC-HFW5200D,DH-IPC-HDBW5200,DH-IPC-HDB5200,DH-IPC-HDBW5202,DH-IPC-HDB5202,DH-IPC-HFW4200E,DH-IPC-HFW4200S,DH-IPC-HDBW4200E,DH-IPC-HDW4200S,DH-IPC-HDW4200C,DH-IPC-HDB4200C,DH-IPC-HDB4200F-PT,DH-IPC-HF5100,DH-IPC-HFW5100E-VF,DH-IPC-HFW5100E-Z,DH-IPC-HFW5100C,DH-IPC-HFW5102C,DH-IPC-HFW5100C-L,DH-IPC-HFW5100-IRA,DH-IPC-HFW5100D,DH-IPC-HDBW5100,DH-IPC-HDB5100,DH-IPC-HFW4100E,DH-IPC-HFW4100S,DH-IPC-HDBW4100E,DH-IPC-HDW4100S,DH-IPC-HDW4100C,DH-IPC-HDB4100C,DH-IPC-HDB4100F-PT

simieski commented 4 years ago

Hi @pnbruckner, so am I right thinking these are the choices now ?

  • Upstream amcrest python implements a bug workaround, if thats what HA is utilising - but not prefered. Would have to be URLencoded != notworking, then try URLNotEncoded so the workaround is not default.
  • HA implements an optional polling, or maybe even switches from Event to polling if it gets a "Bad response" on Event?
  • Older Amcrest and Dahua become unsupported. Circa ~2015 ish is my FW.

Most appreciative of your support to work through, but I guess you have a decision to make :)

As mentioned I have a much new camera on the way as recently moved house, so quite willing to try new and old cameras.

Hi there, was a decision made on this to not support older cameras? It's a real shame, as I bought two IPM-721S in 2018 and they are still selling them now. I'm on firmware version 2.420.AC00.16.R, build : 2016-09-09 and having this exact same issue.

Perhaps I should write a warning on the Amcrest HA docs page? https://www.home-assistant.io/integrations/amcrest/.

Reading above it looks like the two suggested workarounds don't solve the problem. Is that the correct interpretation please?

jamesarbrown commented 4 years ago

Perhaps I should write a warning on the Amcrest HA docs page? https://www.home-assistant.io/integrations/amcrest/.

Tried that, seemed to be very bureacratic and difficult, but I am not used to using git to do these pulls.

I dumped it off amcrestHA, bodged a amcrest2mqtt script and took the info that way.

pnbruckner commented 4 years ago

I have another Amcrest PR pending (to make changing settings more robust), but after that is done I'll try to work on adding a motion_detected_polled option as described above. As I said, trying to figure out when to fall back to polling would be non-trivial, error prone and difficult to fully test. Making it a config item will give the user the ability to decide if/when to use it.

pnbruckner commented 4 years ago

I believe I have it working. It was a little more involved, but what isn't?! I'll try to get PRs (code & doc) submitted soon. Luckily this doen't touch the same files as my other PR, so there should be no problem submitting them both at the same time.

simieski commented 4 years ago

Fantastic news. Thanks so much. Will keep an eye out for it.

MYeager1967 commented 4 years ago

Is there a way to remove the camera part of this as a custom component? I only need the motion detection portion of this as the camera part is creating no end of errors. I'm trying to use the AD110 doorbell and I can get a good video feed anywhere but this integration.