search

agsh / onvif

ONVIF node.js implementation
http://agsh.github.io/onvif/
MIT License
692 stars 234 forks source link

Add action string to Content-Type in ONVIF HTTP Request Header, fixes for PTZSpaces and RelativePTZ #226

Closed RogerHardiman closed 1 year ago

RogerHardiman commented 2 years ago

It looks like SOAP 1.2 requires ths field and 1 ONVIF camera requires it The code is not very efficient. It parses the XML to be sent to generate the action string. It would be better for action strings to be hard coded throughout the library

RogerHardiman commented 2 years ago

Andrew, Chris, could you take a look at this change please.

The SOAP standard allows you to pass the API function in a HTTP header as well as it being part of the XML payload. Eg the HTTP Header includes action="http://www.onvif.org/onvif/ptz/ContinuousMove" or actopn="http://www.onvif.org/onvif/device/GetCapabilities"

The idea is that a XML SOAP server could read the HTTP header and then decide which worker service to pass the XML to without having to parse the entire XML payload and evaluating the namespaces.

In my ONVIF Server projects I see action="..." coming in from lots of VMS systems and from ODM too. But this library does not generate them.

So far all cameras we have used have ignored the lack of action="..." string in the HTTP header. But now I have a camera that requires it.

So we need to add action="..." to the http header of every ONVIF request. So this code computes the action="..." header by parsing the XML we are about to send out. That means it is not very efficient but it does mean that it will work all the time. But I've left the ability in the code for us to hard code action strings in each ONVIF function if we wanted to remove a tiny bit of CPU usage. For me, I'd probably do that in the PTZ commands as they are the ones where you can have a lot of ONVIF messages. For everything else, a few CPU cycles more on each ONVIF command will not make any difference.

So let me know if you think the code is OK and I'll merge into master

agsh commented 2 years ago

@RogerHardiman Hello! I think that everything is ok! I've just point out some syntax questions in the review, they are not mandatory at all

agsh commented 2 years ago

And again and again :smile: I'm looking for the node.js jobs with relocation from Russia :laughing: My LinkedIn: https://www.linkedin.com/in/laptev/

RogerHardiman commented 1 year ago

Hi @agsh. I've made the suggested changes for adding the HTTP Header. I see this PR has also picked up recent changes to PTZSpaces and RelativePTZ commands which I'm using on a project.

agsh commented 1 year ago

@RogerHardiman Hello! Great PR, thanks a lot! I'm waiting for your merge to master :) I'll also make the changes in HTTP headers and PTZ commands in new version here: https://github.com/agsh/onvif/blob/v1/src/onvif.ts#L276 https://github.com/agsh/onvif/blob/v1/src/ptz.ts#L373

agsh commented 1 year ago

@RogerHardiman Hello again! Can I merge your PR to maser branch? What do you think?

RogerHardiman commented 1 year ago

Hi Thankyou for the reminder to merge my branch into 'master' I have just clicked on the 'merge' button.

I had several fixes in the 'roger' branch which fix problems talking to several makes of ONVIF camera. I also added in some code for the optional Click To Centre ONVIF feature although I think only Axis and Hik support it (Bosch and 360 Vision do not support it)