-
Notifications
You must be signed in to change notification settings - Fork 52
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
Minor cleanups and some new example programs. #67
base: master
Are you sure you want to change the base?
Conversation
Loosen restrictions.
Improvements to share common code. Fix threaded version to open window in main thread.
Forgot to commit with previous. New method: stop_stream.
New camera method: is_logged_in
New example program to set ftp parameters.
get_info calls all the info methods. snap will snap a single frame image.
Getting occasional garbage back from the cameras which causes the json conversion to crap out. Print some info when this happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @phbcanada
Thank you for the contribution! This looks quite good and we should easily be able to merge it :)
Just one question about the version specifier inside the setup.py
file.
'numpy>=1.19.4', | ||
'opencv-python>=4.4.0.46', | ||
'Pillow>=8.0.1', | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I agree with the inclusive ordered comparison >=
. Minor versions could introduce breaking changes at some point. I would rather stick with a specific version and in this case update the specific version manually to a later release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The install failed on one of my systems so this was the easy fix. Not a big issue for me.
Could we also use conventional commits on the PR title? I know this repository doesn't have any real guidelines on this and in the past it's been quite lax, but I think it is good practice. you can change the title to something like |
I haven't used git/github for several years. Not really up on current conventions. Sorry. |
np :) |
The GetAbility command returns a property named "scheduleVersion" which indicates which API calls should be used. The new ones have a "V20" tacked onto the command name.
Any chances to get this merged? The set_network_ftp() method would be really helpful. |
There looks to be a simple requested change that still needs to be done. |
Improve set_osd call. Extend FTP setting support. Add set_network_ntp Add set_network_email Fixes to add_user and modify_user.
GetAlarm is not supported on newer cameras.
Correct call is "SetEnc" not "SetRec". Argument types changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some minor changes. The setup.py
is a bit concerning, IMO it needs to be fixed versions.
body = [{"cmd": "SetOsd", # "action": 1, | ||
"param": { | ||
"Osd": { | ||
"bgcolor": bg_color, | ||
# "bgcolor": bg_color, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are these commented out?
}}}] | ||
# print("SetOsd:", json.dumps(body[0], indent=4)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just delete?
@@ -25,9 +25,127 @@ def set_net_port(self, http_port: float = 80, https_port: float = 443, media_por | |||
"rtspPort": rtsp_port | |||
}}}] | |||
self._execute_command('SetNetPort', body, multi=True) | |||
print("Successfully Set Network Ports") | |||
# print("Successfully Set Network Ports") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# print("Successfully Set Network Ports") | |
print("Successfully Set Network Ports") |
r_data = self._execute_command('ModifyUser', body)[0] | ||
if r_data["value"]["rspCode"] == 200: | ||
# print(f"modify user: {username}\nCamera responded with: {json.dumps(r_data, indent=4)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# print(f"modify user: {username}\nCamera responded with: {json.dumps(r_data, indent=4)}") |
body = [{"cmd": "Login", "action": 0, | ||
"param": {"User": {"userName": self.username, "password": self.password}}}] | ||
param = {"cmd": "Login", "token": "null"} | ||
response = Request.post(self.url, data=body, params=param) | ||
if response is not None: | ||
# print("LOGIN GOT: ", response.text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# print("LOGIN GOT: ", response.text) |
@@ -64,28 +68,47 @@ def login(self) -> bool: | |||
:return: bool | |||
""" | |||
try: | |||
# Suppress only the single warning from urllib3 needed. | |||
requests.packages.urllib3.disable_warnings(category=InsecureRequestWarning) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe make this optional?
or what about just leaving this to the client to set this as an env variable
PYTHONWARNINGS="ignore:Unverified HTTPS request"
# print("Login success") | ||
else: | ||
# print(self.token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the comments or restore the print of "Login success?"
The threaded streaming example program wasn't working so I fixed it.
The new "set_ftp" example is useful for me since I use FTP to record all my camera motion videos.
The "snap" example is useful for grabbing snapshots from each camera.
I use hostnames instead of IP addresses for my cameras. Just put the IP addresses in /etc/hosts.