hawkeye64 / onvif-nvt

ONVIF library for NVT (Network Video Transmitter) devices.
MIT License
76 stars 23 forks source link

Something seems not right #370

Open hildoer opened 4 years ago

hildoer commented 4 years ago

While trying to create automated tests, which reset process state between tests, I found issues connecting to devices. I dug into this code and found this line inside node_modules/onvif-nvt/dist/onvif-nvt.js:

module.exports = new OnvifManager();

Inside the OnvifManager class is a field cameras, which gets a cache of every connected camera, by host name. So, if I want to kill a connection to a camera, there is no way to do it. I can overwrite the connection, but not delete it, reset it,etc.

Also, the .add() method to add discovery is also permanent for the process. No way to properly test code around this manager with and without discovery except by manually breaking the published interface and calling something like:

const OnvifManager = require( 'onvif-nvt' ); OnvifManager.add( 'discovery' ); delete OnvifManager.discovery; Is that right? Is that the intended code pattern for this library? How would one properly write tests on their code if it is dependent on onvif-nvt since onvif-nvt only supports immutable global state?

Any advice on how to properly interface with onvif-nvt when doing test-driven-development would be awesome!

One other thing, the code is riddled with console.log and console.error lines all over. Its polluting stdout/err. Any plans to make a proper logging facility, or at least be able to disable straight to console logs?

Thank you!

hawkeye64 commented 3 years ago

@hildoer Admittedly, it was my first big library when I was learning js. These days, I am much more learned. You are right, tho. We need to break the "intended code pattern for this library" and do it better. I would love some help, since I don't have a lot of the cameras available to me any longer. Would you be up for it? If I created a "next" branch?

hawkeye64 commented 3 years ago

I pushed "dev" and "next" branches. I will see if I can find time this weekend to work on it a bit. v1.0.0 release, will be a breaking changes release.