Amcrest integration using http with port 443

jakommo commented 4 years ago

The problem

I setup the amcrest component and it resulted in warnings like the following

2020-05-06 22:05:55 WARNING (SyncWorker_14) [amcrest.http] <None:None> Trying again due to error: ConnectionError(ProtocolError('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer')))

and did not make my cam available in HA.

After enabling debug logging for amcrest.http I saw the following request being made:

2020-05-06 22:07:57 DEBUG (SyncWorker_13) [amcrest.http] <None:None> Trying Basic Authentication
2020-05-06 22:07:57 DEBUG (SyncWorker_13) [amcrest.http] <None:None> HTTP query 15: http://192.188.15:443/cgi-bin/magicBox.cgi?action=getMachineName
2020-05-06 22:07:57 DEBUG (SyncWorker_13) [amcrest.http] <None:None> Running query 15 attempt 1

Note the http:// but using port 443.

After removing port 443 from my config and using default port 80, it started to work.

My assumption was that using port: 443 would also make it connect via https and it took me a while to figure out that this was the cause of the failing connections.

I would still prefer to connect via https, but haven't seen any option that would enable this. Does the integration support https at all? If not then I can do a PR to update the doc to make this more clear htat using port 443 will not make it use https. Not sure if this is a bug or feature request.

Environment

Home Assistant Core release with the issue: 109.4
Last working Home Assistant Core release (if known): -
Operating environment (Home Assistant/Supervised/Docker/venv): Home Assistant Supervised
Integration causing this issue: Amcrest
Link to integration documentation on our website: https://www.home-assistant.io/components/camera.amcrest/

Problem-relevant `configuration.yaml`

amcrest:
  - host: 192.168.1.15
    name: cam2
    port: 443
    username: !secret xxx
    password: !secret xxx
    binary_sensors:
      - motion_detected
      - online

Traceback/Error logs

2020-05-06 22:07:57 DEBUG (SyncWorker_13) [amcrest.http] <None:None> Trying Basic Authentication
2020-05-06 22:07:57 DEBUG (SyncWorker_13) [amcrest.http] <None:None> HTTP query 15: http://192.188.15:443/cgi-bin/magicBox.cgi?action=getMachineName
2020-05-06 22:07:57 DEBUG (SyncWorker_13) [amcrest.http] <None:None> Running query 15 attempt 1

Additional information

dshokouhi commented 4 years ago

I am not sure if https is supported with the amcrest integration, so this may be a feature request.

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

amcrest documentation amcrest source _{^{(message by IssueLinks)}}

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! _{^{(message by CodeOwnersMention)}}

pnbruckner commented 4 years ago

The amcrest PYPI package that the HA amcrest integration uses does provide a way to specify the use of HTTPS, as well as to specify whether or not SSL certificates will be verified. However, the amcrest integration, to my knowledge, has never supported those features. So, yes, this would be a feature request.

frenck commented 4 years ago

Thank you for reaching out. We use GitHub for tracking issues, not for providing support or tracking feature requests.

If you want to suggest a feature, you should try our Community Forum: Feature Requests.

If you have additional questions, feel free to join our Discord chat server.

Thanks! 👍

home-assistant / core