caldav binding 15/11 #3422

querdenker2k · 2015-11-15T10:17:36Z

PR for caldav-binding in one commit

hakan42 · 2015-11-15T15:16:36Z

@teichsta , could you please assign this to me for review?

hakan42 · 2015-11-15T15:26:44Z

....caldav-command/src/main/java/org/openhab/binding/caldav_command/internal/CalDavBinding.java

+		} else if (type == CalDavType.DISABLE) {
+			// ok
+		} else {
+			logger.warn("unhandled type: " + type);


logging: Please use slf4j parametrization instead of string concatenation

teichsta · 2015-11-15T16:06:52Z

could you please assign this to me for review?

i've invited you to the openHAB organisation which is the prerequisite …

hakan42 · 2015-11-15T16:45:55Z

Thank you, invitation accepted.

querdenker2k · 2015-11-15T20:35:45Z

Thanks for review. Sorry for simple mistakes (Eclipse is not my default IDE)

hakan42 · 2015-11-15T20:39:26Z

You're welcome. Honestly, I had to re-train myself too as I am a IntelliJ user in my day job :)

hakan42 · 2015-11-15T20:45:13Z

Could you add a README.md with a one-line reason why you have to use specific library versions? I am sure this will be an unexpected thing for others too.

Maybe you could even use the README templates as they are used in OH2, then you would already be well prepared for migration :)

querdenker2k · 2015-11-15T21:09:10Z

I am IntelliJ user as well.
I do not added the OH2 template, because i have to change the binding for OH2 as well. (havn't i?)

kaikreuzer · 2015-11-15T21:15:02Z

I do not added the OH2 template, because i have to change the binding for OH2 as well. (havn't i?)

No, you don't have to. OH1 bindings will work just fine with OH2 as well. Only if you want to use features of the new APIs such as discovery, you might want to think about a migration. Otherwise, just maintain the OH1 binding and all is fine :-)

querdenker2k · 2015-11-16T16:49:02Z

Yes i know that they are working. But OH2 Bindings have another namespace and the README template contains Things, Discovery etc., therefore i didn't used the OH2 template.

kaikreuzer · 2015-11-16T16:51:10Z

Right, so for OH1 bindings there is no need (yet) to add a README.md - the documentation is expected on the wiki (which we might move to a README.md in the future, but that will then be done for all at once).

hakan42 · 2015-11-16T18:44:00Z

@querdenker2k : I am not so sure about the binding name for "caldavPersonal". You mention that is is meant to simulate presence detection. Do you think it would be possible to split this PR into two parts, one with the io and command part, and another with the "personal" part?

The first part looks good to me (still need to compile and run a few tests on my OH2 instance) but I'm not really comfortable with the other part.

On the other hand, I am well aware that finding good names is one of the hardest problems in computing 😄

querdenker2k · 2015-11-16T18:50:14Z

No you can use caldavCommand for presence simulation. I currently use it to regulate my heaters. This binding is just interested in the event description.
The caldavPersonal is for querying events. To show your upcoming events. This binding is not interested in the event description, but all other fields.

hakan42 · 2015-11-17T17:31:02Z

....caldav-command/src/main/java/org/openhab/binding/caldav_command/internal/CalDavBinding.java

+				continue;
+			}
+
+			final List<EventUtils.EventContent> parseContent = EventUtils.parseContent(calDavEvent, item);


Could you please put this for loop immediately after the "try"? To avoid using "continue" in loops?

I change it, but my intention to do it this is way is: I would like to see that the item could not be found in the registry and because of this it has to be skipped. If the try-catch is around of a big block it is not directly visible. But nevertheless i change it.

hakan42 · 2015-11-17T18:26:47Z

...caldav-personal/src/main/java/org/openhab/binding/caldav_personal/internal/CalDavConfig.java

+
+	public CalDavConfig(List<String> calendar, Type type, int eventNr,
+			Value value) {
+		super();


CalDavConfig just "implements" BindingConfig. Do we need a super() here?

hakan42 · 2015-11-17T19:08:46Z

Actually ok then, leave the readme.txt files in the source. If @kai thinks it necessary to remove them this can be done in a big commit over all addons at once.

querdenker2k · 2015-11-19T17:42:25Z

Maybe you can help me. I have just problems with eclipse and osgi. Every time i have to reload the osgi runtime (in preferences). But now with the current trunk i got this exception for every bundle. What is going wrong?

osgi> osgi> !SESSION 2015-11-19 18:39:22.612 -----------------------------------------------
eclipse.buildId=unknown
java.version=1.8.0_66
java.vendor=Oracle Corporation
BootLoader constants: OS=win32, ARCH=x86_64, WS=win32, NL=de_DE
Framework arguments:  -application
Command-line arguments:  -application -data C:\Users\Robert\Java-Workspace\openhab\openhab-querdenker2k/../runtime-org.openhab.runtime.product.product -dev file:C:/Users/Robert/Java-Workspace/openhab/openhab-querdenker2k/.metadata/.plugins/org.eclipse.pde.core/openHAB Runtime/dev.properties -os win32 -ws win32 -arch x86_64 -consoleLog -console

!ENTRY org.openhab.binding.caldav-command 4 0 2015-11-19 18:39:25.961
!MESSAGE FrameworkEvent ERROR
!STACK 0
org.osgi.framework.BundleException: The activator org.openhab.binding.caldav_command.internal.CalDavActivator for bundle org.openhab.binding.caldav-command is invalid
    at org.eclipse.osgi.framework.internal.core.AbstractBundle.loadBundleActivator(AbstractBundle.java:172)
    at org.eclipse.osgi.framework.internal.core.BundleContextImpl.start(BundleContextImpl.java:679)
    at org.eclipse.osgi.framework.internal.core.BundleHost.startWorker(BundleHost.java:381)
    at org.eclipse.osgi.framework.internal.core.AbstractBundle.resume(AbstractBundle.java:390)
    at org.eclipse.osgi.framework.internal.core.Framework.resumeBundle(Framework.java:1176)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.resumeBundles(StartLevelManager.java:559)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.resumeBundles(StartLevelManager.java:544)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.incFWSL(StartLevelManager.java:457)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.doSetStartLevel(StartLevelManager.java:243)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.dispatchEvent(StartLevelManager.java:438)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.dispatchEvent(StartLevelManager.java:1)
    at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230)
    at org.eclipse.osgi.framework.eventmgr.EventManager$EventThread.run(EventManager.java:340)
Caused by: java.lang.ClassNotFoundException: org.openhab.binding.caldav_command.internal.CalDavActivator
    at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:501)
    at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:421)
    at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:412)
    at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.loadClass(DefaultClassLoader.java:107)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
    at org.eclipse.osgi.internal.loader.BundleLoader.loadClass(BundleLoader.java:340)
    at org.eclipse.osgi.framework.internal.core.BundleHost.loadClass(BundleHost.java:229)
    at org.eclipse.osgi.framework.internal.core.AbstractBundle.loadBundleActivator(AbstractBundle.java:165)
    ... 12 more
Root exception:
java.lang.ClassNotFoundException: org.openhab.binding.caldav_command.internal.CalDavActivator
    at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:501)
    at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:421)
    at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:412)
    at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.loadClass(DefaultClassLoader.java:107)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
    at org.eclipse.osgi.internal.loader.BundleLoader.loadClass(BundleLoader.java:340)
    at org.eclipse.osgi.framework.internal.core.BundleHost.loadClass(BundleHost.java:229)
    at org.eclipse.osgi.framework.internal.core.AbstractBundle.loadBundleActivator(AbstractBundle.java:165)
    at org.eclipse.osgi.framework.internal.core.BundleContextImpl.start(BundleContextImpl.java:679)
    at org.eclipse.osgi.framework.internal.core.BundleHost.startWorker(BundleHost.java:381)
    at org.eclipse.osgi.framework.internal.core.AbstractBundle.resume(AbstractBundle.java:390)
    at org.eclipse.osgi.framework.internal.core.Framework.resumeBundle(Framework.java:1176)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.resumeBundles(StartLevelManager.java:559)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.resumeBundles(StartLevelManager.java:544)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.incFWSL(StartLevelManager.java:457)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.doSetStartLevel(StartLevelManager.java:243)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.dispatchEvent(StartLevelManager.java:438)
    at org.eclipse.osgi.framework.internal.core.StartLevelManager.dispatchEvent(StartLevelManager.java:1)
    at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230)
    at org.eclipse.osgi.framework.eventmgr.EventManager$EventThread.run(EventManager.java:340)
18:39:25.965 [DEBUG] [.b.http.internal.HttpActivator:34   ] - HTTP binding has been started.

hakan42 · 2015-11-20T13:55:46Z

sorry, maybe @kaikreuzer can help with the OSGI stuff, I am only a stupid Java hacker 😄

kaikreuzer · 2015-11-20T14:05:27Z

No idea, looks as if the class file cannot be found. Did you already try a "clean project" to make it recompile?

querdenker2k · 2015-11-21T12:00:44Z

Ok, all changes are done. I will test it for a few days and the commit it.

hakan42 · 2015-11-23T15:40:53Z

@querdenker2k , this branch is showing conflicts again, very possibly because of commits to bundles/binding/pom.xml that got merged in the meantime.

Could you please update the PR? Just fixing the conflict and pushing to this branch should be enough.

querdenker2k · 2015-11-25T17:39:08Z

I have pulled and merged, but git wants to add all changes from the remote head as well? Is this fine?

hakan42 · 2015-11-25T18:00:56Z

You should have run "git rebase" instead of "pull" and "merge".

If this turns out to be too much work, you can just run:

"git stash" to make a copy of your files (actually, maybe just make a zip archive too, just to be sure 😄 ),
"git reset --hard HEAD" to make sure that your head is identical to what the openhab repository is,
"git stash pop" to get the copy of your files back
fix any conflicts that may happen, like bindings/pom.xml or io/pom.xml
"git add"
"git commit"
and, finally, "git push --force"

The pull request should be automatically updated.

querdenker2k · 2015-11-25T18:28:34Z

but with this commands its not getting the changes from the remote head

hakan42 · 2015-11-25T18:37:18Z

o.k., maybe a

"git pull origin master"
"git rebase origin/master"

Honestly, I don't have a good idea, so if you don't have pushed anything from your merge yet, maybe just throw the clone away, get a fresh copy from github, and try again to rebase?

hakan42 · 2015-11-25T18:46:30Z

Another minor nitpick: Would you mind to call all your logger objects

private final Logger logger = LoggerFactory.getLogger(YourClass.class)

Especially your internal classes have a few logger objects which are called LOG (which is not really correct from a Java style point of view).

querdenker2k · 2015-11-26T16:50:27Z

All the time in every project i wrote LOG like this, but you're right, the coding style says log. thx.

All changes are done actually.

hakan42 · 2015-11-26T18:12:56Z

@querdenker2k , your branch is by now totally confused because git is sometimes evil 😈

The source of your own files look o.k. to me. Would you mind to create a fresh branch + PR and add only your own files to it? And close this PR?

After that, I would be happy to merge your branch so we have caldav support in 1.8.0 after all your good work 😄

querdenker2k · 2015-11-26T18:59:24Z

ok, this is the new pr, i hope that fits.
#3468

caldav binding

e260d69

querdenker2k mentioned this pull request Nov 15, 2015

New CalDAV Binding, Used for scheduling and presence detection #2215

Closed

hakan42 reviewed Nov 15, 2015
View reviewed changes

teichsta assigned hakan42 Nov 15, 2015

review fixes

a583de4

add readme

51cf439

hakan42 reviewed Nov 17, 2015
View reviewed changes

hakan42 added new-binding-action-service review-in-progress labels Nov 19, 2015

remove creating new threads

668a1a7

merge

7b0a944

querdenker2k added 2 commits November 26, 2015 17:41

fix merge problems

03547b7

change LOG to log

592c5e9

querdenker2k closed this Nov 26, 2015

hakan42 added stale and removed new-binding-action-service review-in-progress labels Nov 26, 2015

querdenker2k deleted the caldav-15-11 branch February 18, 2016 19:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

caldav binding 15/11 #3422

caldav binding 15/11 #3422

querdenker2k commented Nov 15, 2015

hakan42 commented Nov 15, 2015

hakan42 Nov 15, 2015

teichsta commented Nov 15, 2015

hakan42 commented Nov 15, 2015

querdenker2k commented Nov 15, 2015

hakan42 commented Nov 15, 2015

hakan42 commented Nov 15, 2015

querdenker2k commented Nov 15, 2015

kaikreuzer commented Nov 15, 2015

querdenker2k commented Nov 16, 2015

kaikreuzer commented Nov 16, 2015

hakan42 commented Nov 16, 2015

querdenker2k commented Nov 16, 2015

hakan42 Nov 17, 2015

querdenker2k Nov 17, 2015

hakan42 Nov 17, 2015

hakan42 commented Nov 17, 2015

querdenker2k commented Nov 19, 2015

hakan42 commented Nov 20, 2015

kaikreuzer commented Nov 20, 2015

querdenker2k commented Nov 21, 2015

hakan42 commented Nov 23, 2015

querdenker2k commented Nov 25, 2015

hakan42 commented Nov 25, 2015

querdenker2k commented Nov 25, 2015

hakan42 commented Nov 25, 2015

hakan42 commented Nov 25, 2015

querdenker2k commented Nov 26, 2015

hakan42 commented Nov 26, 2015

querdenker2k commented Nov 26, 2015

caldav binding 15/11 #3422

caldav binding 15/11 #3422

Conversation

querdenker2k commented Nov 15, 2015

hakan42 commented Nov 15, 2015

hakan42 Nov 15, 2015

Choose a reason for hiding this comment

teichsta commented Nov 15, 2015

hakan42 commented Nov 15, 2015

querdenker2k commented Nov 15, 2015

hakan42 commented Nov 15, 2015

hakan42 commented Nov 15, 2015

querdenker2k commented Nov 15, 2015

kaikreuzer commented Nov 15, 2015

querdenker2k commented Nov 16, 2015

kaikreuzer commented Nov 16, 2015

hakan42 commented Nov 16, 2015

querdenker2k commented Nov 16, 2015

hakan42 Nov 17, 2015

Choose a reason for hiding this comment

querdenker2k Nov 17, 2015

Choose a reason for hiding this comment

hakan42 Nov 17, 2015

Choose a reason for hiding this comment

hakan42 commented Nov 17, 2015

querdenker2k commented Nov 19, 2015

hakan42 commented Nov 20, 2015

kaikreuzer commented Nov 20, 2015

querdenker2k commented Nov 21, 2015

hakan42 commented Nov 23, 2015

querdenker2k commented Nov 25, 2015

hakan42 commented Nov 25, 2015

querdenker2k commented Nov 25, 2015

hakan42 commented Nov 25, 2015

hakan42 commented Nov 25, 2015

querdenker2k commented Nov 26, 2015

hakan42 commented Nov 26, 2015

querdenker2k commented Nov 26, 2015