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

1376 - Initial contribution of ZoneMinder Binding #1376

Closed
wants to merge 109 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@Mr-Eskildsen
Contributor

Mr-Eskildsen commented Nov 1, 2016

First version of a ZoneMinder Binding . It handles basic operations, eg. trigger zoneminder from OpenHAB, and monitors health status of ZoneMinder Server from OpenHAB.

MartinEskildsen added some commits Oct 21, 2016

@Mr-Eskildsen

This comment has been minimized.

Show comment
Hide comment
@Mr-Eskildsen

Mr-Eskildsen Jan 23, 2017

Contributor

This jar is what I hope would be the final build before release. The functionallity that was planned is there and I haven't seen any problems in my setup.
org.openhab.binding.zoneminder-2.0.0-SNAPSHOT.zip

Contributor

Mr-Eskildsen commented Jan 23, 2017

This jar is what I hope would be the final build before release. The functionallity that was planned is there and I haven't seen any problems in my setup.
org.openhab.binding.zoneminder-2.0.0-SNAPSHOT.zip

@Mr-Eskildsen

This comment has been minimized.

Show comment
Hide comment
@Mr-Eskildsen

Mr-Eskildsen Jan 24, 2017

Contributor

This build does address some bugs during initialisation, which will likely cause one or more monitors not to be initialised correct. Furthermore the discovery service has been rewritten, now it works like the ZWave Binding, eg. you will have to create a Bridge Thing, and then the monitors will be automatically discovered.
org.openhab.binding.zoneminder-2.1.0-SNAPSHOT.zip

Contributor

Mr-Eskildsen commented Jan 24, 2017

This build does address some bugs during initialisation, which will likely cause one or more monitors not to be initialised correct. Furthermore the discovery service has been rewritten, now it works like the ZWave Binding, eg. you will have to create a Bridge Thing, and then the monitors will be automatically discovered.
org.openhab.binding.zoneminder-2.1.0-SNAPSHOT.zip

@Mr-Eskildsen

This comment has been minimized.

Show comment
Hide comment
@Mr-Eskildsen

Mr-Eskildsen Jan 25, 2017

Contributor

@kaikreuzer: I would like to reqeust a code review for this Binding.
I have a few comments my self:

  • I have merged with the 2.1.0, so it should be pretty much aligned to the master branch.
  • It claims that some files in other bindings has changed. I don't understand why and/or how that has happended. I have ONLY changed the pom.xml outside the zoneminder binding. That did cause an merge conflict because ofd the new ZWay binding, That has been solved. If I can/should do something please let me know.
  • If you see something that should obvious be changed throughout all files, please let me know, instead of you wasting your time looking for the same problem in 10 different files.
  • The binding has been actively tested in my own setup and in another users setup. I have seen some stability problems, which should have been solved by now (that is the reason for taking so long time).

If you think any of the above comments requires me to do some additional changes, please let me know up front.

UPDATE: I looked a bit more into to the merge conflicts. I now believe that I have managed to align my fork with the upstream&master. If not, then please let me know .

Contributor

Mr-Eskildsen commented Jan 25, 2017

@kaikreuzer: I would like to reqeust a code review for this Binding.
I have a few comments my self:

  • I have merged with the 2.1.0, so it should be pretty much aligned to the master branch.
  • It claims that some files in other bindings has changed. I don't understand why and/or how that has happended. I have ONLY changed the pom.xml outside the zoneminder binding. That did cause an merge conflict because ofd the new ZWay binding, That has been solved. If I can/should do something please let me know.
  • If you see something that should obvious be changed throughout all files, please let me know, instead of you wasting your time looking for the same problem in 10 different files.
  • The binding has been actively tested in my own setup and in another users setup. I have seen some stability problems, which should have been solved by now (that is the reason for taking so long time).

If you think any of the above comments requires me to do some additional changes, please let me know up front.

UPDATE: I looked a bit more into to the merge conflicts. I now believe that I have managed to align my fork with the upstream&master. If not, then please let me know .

@Mr-Eskildsen

This comment has been minimized.

Show comment
Hide comment
@Mr-Eskildsen

Mr-Eskildsen Jan 26, 2017

Contributor

New build, primarily additional logging and improved handling of Thing ONLINE / OFFLINE status
org.openhab.binding.zoneminder-2.1.0-SNAPSHOT.zip

Contributor

Mr-Eskildsen commented Jan 26, 2017

New build, primarily additional logging and improved handling of Thing ONLINE / OFFLINE status
org.openhab.binding.zoneminder-2.1.0-SNAPSHOT.zip

@Mr-Eskildsen

This comment has been minimized.

Show comment
Hide comment
@Mr-Eskildsen

Mr-Eskildsen Jan 27, 2017

Contributor

New build that does correctly detect a ZoneMinder Instance when it run any language (and not just english).
org.openhab.binding.zoneminder-2.1.0-SNAPSHOT.zip

Contributor

Mr-Eskildsen commented Jan 27, 2017

New build that does correctly detect a ZoneMinder Instance when it run any language (and not just english).
org.openhab.binding.zoneminder-2.1.0-SNAPSHOT.zip

@whopperg

This comment has been minimized.

Show comment
Hide comment
@whopperg

whopperg Jan 29, 2017

Hey @Mr-Eskildsen ,

just checked out the latest release and im getting a lot of Infos in my openhab.log:
2017-01-29 15:58:46.394 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 15:59:02.553 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 15:59:17.863 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 15:59:33.177 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 15:59:48.497 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:00:03.808 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:00:19.119 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:00:34.429 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:00:49.746 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:01:05.066 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE'
Maybe some leftover debug code?

Otherwhise, seems to work jsut fine!

Cheers

whopperg commented Jan 29, 2017

Hey @Mr-Eskildsen ,

just checked out the latest release and im getting a lot of Infos in my openhab.log:
2017-01-29 15:58:46.394 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 15:59:02.553 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 15:59:17.863 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 15:59:33.177 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 15:59:48.497 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:00:03.808 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:00:19.119 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:00:34.429 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:00:49.746 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE' 2017-01-29 16:01:05.066 [INFO ] [andler.ZoneMinderServerBridgeHandler] - [BRIDGE (914be859)]: Bridge status changed from 'ONLINE' to 'ONLINE'
Maybe some leftover debug code?

Otherwhise, seems to work jsut fine!

Cheers

@Mr-Eskildsen

This comment has been minimized.

Show comment
Hide comment
@Mr-Eskildsen

Mr-Eskildsen Jan 29, 2017

Contributor

Hi @whopperg,

Actually it is a small bug that has been corrected. I was trying only to log when state has changed. I didn't build a new a jar, but since it has been pinpointed a couple of times I will provide a build that corresponds to the latest source :-)

org.openhab.binding.zoneminder-2.1.0-SNAPSHOT.zip

Contributor

Mr-Eskildsen commented Jan 29, 2017

Hi @whopperg,

Actually it is a small bug that has been corrected. I was trying only to log when state has changed. I didn't build a new a jar, but since it has been pinpointed a couple of times I will provide a build that corresponds to the latest source :-)

org.openhab.binding.zoneminder-2.1.0-SNAPSHOT.zip

Updated zoneminder4j so that ZoneMinder is correctly detected when it is
running other language than english

Update .classpath

 Changes to be committed:
	modified:   addons/binding/org.openhab.binding.zoneminder/.classpath
	modified:   addons/binding/org.openhab.binding.zoneminder/META-INF/MANIFEST.MF
	new file:   addons/binding/org.openhab.binding.zoneminder/lib/zoneminder4j-0.9.7.jar
@kaikreuzer

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Jan 31, 2017

Member

Thanks @Mr-Eskildsen!

There seems to be something very wrong with your branch, though. It shows 147 changed files, many not related to your binding.
Note that you should actually NEVER merge master or other branches into your feature branch - instead, you should regularly rebase your feature branch onto the latest master.
I'd suggest you first clean up your branch before I do another review.

Member

kaikreuzer commented Jan 31, 2017

Thanks @Mr-Eskildsen!

There seems to be something very wrong with your branch, though. It shows 147 changed files, many not related to your binding.
Note that you should actually NEVER merge master or other branches into your feature branch - instead, you should regularly rebase your feature branch onto the latest master.
I'd suggest you first clean up your branch before I do another review.

@kaikreuzer

This comment has been minimized.

Show comment
Hide comment
@kaikreuzer

kaikreuzer Feb 2, 2017

Member

Replaced by #1812.

Member

kaikreuzer commented Feb 2, 2017

Replaced by #1812.

@kaikreuzer kaikreuzer closed this Feb 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment