[Loxone] Added null annotations #2604

ppieczul · 2017-08-27T16:25:37Z

NonNullByDefault was set on all packages except thing factory, which consists of basically single method and results mainly in warning suppression.
Also json deserialization classes were marked nullable by default, because they will get nulls wherever a deserialized json does not contain a specific field. However, for the maps in json classes, I added nonnull for both key and value, as I expect GSON to produce a meaningful map or no map at all.
This resulted in removal of some dead code and redundant null checks, but also adding missing null checks.
To get rid of all compiler warnings, I added suppression only at local variables taking values from legacy calls. Also added some more thread-safe constructions with local object references instead accessing compound data that theoretically could be modified outside of the scope, to eliminate warnings about possible null accesses. They should not happen in reality, but I don't think this is resulting in a bad code.

Signed-off-by: Pawel Pieczul pieczul@gmail.com (github: ppieczul)

ppieczul · 2017-09-02T21:20:13Z

@kaikreuzer @martinvw - any chance for a review? Thanks

ppieczul · 2017-09-09T10:24:49Z

@kaikreuzer @martinvw - any chance for a review? Thanks

kaikreuzer · 2017-09-11T08:46:34Z

@ppieczul We do not have any final agreement on how to use the null annotations - we would like to have some guidelines that are followed in the same way throughout the codebase, so that it is clear to contributors, what is expected from them. We especially didn't (yet) decide to use any of the "*ByDefault". Please find the current discussion here and feel free to involve yourself in it! (Your PR might be a good reference for pros//cons for the different options).

martinvw · 2017-09-22T12:38:02Z

@ppieczul I believe there a conclusion to the discussion, can you confirm whether this PR already follows those new guidelines.

Thanks!

ppieczul · 2017-09-22T13:29:36Z

I will check it over the weekend. Thanks.

…

On Fri, 22 Sep 2017 at 14:38, Martin van Wingerden ***@***.***> wrote: @ppieczul <https://github.com/ppieczul> I believe there a conclusion <https://github.com/eclipse/smarthome/pull/4275/files#diff-24ba7e763168d1631a43ebdef19bcd5d> to the discussion, can you confirm whether this PR already follows those new guidelines. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/openhab/openhab2-addons/pull/2604#issuecomment-331434968>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJx5fqNIj7UGTRSWvo4BFB6oGkFGarksks5sk6ougaJpZM4PD2Oc> .

ppieczul · 2017-09-23T21:55:55Z

@martinvw I think I need your help here. I made a new installation of eclipse IDE and upon running a debug configuration I am getting unresolved upnp bundle probably due to unresolved apache http.

!ENTRY org.jupnp 4 0 2017-09-23 23:45:35.981
!MESSAGE FrameworkEvent ERROR
!STACK 0
org.osgi.framework.BundleException: Could not resolve module: org.jupnp [29]
  Unresolved requirement: Import-Package: org.apache.http.client
    -> Export-Package: org.apache.http.client; bundle-symbolic-name="org.apache.httpcomponents.httpclient"; bundle-version="4.5.2.v20170210-0925"; version="4.5.2"; uses:="org.apache.http,org.apache.http.auth,org.apache.http.client.methods,org.apache.http.conn,org.apache.http.conn.routing,org.apache.http.cookie,org.apache.http.params,org.apache.http.protocol"
       org.apache.httpcomponents.httpclient [104]
         Unresolved requirement: Import-Package: org.apache.http; version="[4.4.6,4.5.0)"
  Unresolved requirement: Import-Package: org.apache.http.client.methods
    -> Export-Package: org.apache.http.client.methods; bundle-symbolic-name="org.apache.httpcomponents.httpclient"; bundle-version="4.5.2.v20170210-0925"; version="4.5.2"; uses:="org.apache.http,org.apache.http.client.config,org.apache.http.concurrent,org.apache.http.conn,org.apache.http.message,org.apache.http.params"

	at org.eclipse.osgi.container.Module.start(Module.java:444)
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.incStartLevel(ModuleContainer.java:1628)
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.incStartLevel(ModuleContainer.java:1608)
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.doContainerStartLevel(ModuleContainer.java:1579)
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.dispatchEvent(ModuleContainer.java:1522)
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.dispatchEvent(ModuleContainer.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)

This prevents from running bindings that use upnp, for example hue which is enabled in the default configuration. I tried purging eclipse and its hidden folders and reinstalling, but same story.

Do you have any ideas how to overcome this?

Thanks
Pawel

ppieczul · 2017-09-24T12:34:11Z

First, the problem with unresolved references under IDE seems to be random. It depends on how the eclipse starts. Sometimes after performing all the initial activities, the problem exists (as long as eclipse is open). But sometimes, after subsequent eclipse restart, the problem goes away and everything works fine as long as eclipse session is open. I don't plan to investigate it.

@martinvw I think my changes conform to the general idea of having classes non null by default, with Nullable added wherever it is needed. I added package-info files to each package, as this forces future new classes to follow this guideline.

I updated the PR to the latest code and it should be ready for review/merge (if the checks don't fail...)

maggu2810 · 2017-09-24T13:04:56Z

FYI: In ESH we considered not using package-info files as it is hard to understand which classes uses nonnull by default and which not then (as you cannot rely on the class annotation only).

ppieczul · 2017-09-24T15:37:39Z

If this is the direction, let it be. I changed the code to follow it.

ppieczul · 2017-09-25T04:45:29Z

@martinvw @kaikreuzer would it be a good time now to review and merge these changes? Thanks.

ppieczul · 2017-09-27T16:54:54Z

@martinvw: I merged with the top code. Kindly asking for a review and merge. Thanks.

martinvw · 2017-09-27T20:27:34Z

@ppieczul please realise that we have quite some queue and your earlier contributions already jumped the queue, so maybe we can make an arrangement. If you could review some other PR(s) then I will make sure this gets reviewed and merged ASAP. Hereby some (more or less random) suggestions:

I always use the following filters to find PR's which are ready to be reviewed: https://github.com/openhab/openhab2-addons/pulls?page=2&q=is%3Apr+is%3Aopen+-label%3A%22Work+in+progress%22+-label%3A%22Awaiting+feedback%22+-assignee%3Akaikreuzer&utf8=%E2%9C%93

kaikreuzer · 2017-09-27T20:32:22Z

Haha, @ppieczul that's a broad hint that you seem to be a good candidate to help us a bit on reviews. What do you think? It would help making us all faster and reduce the waiting time for everyone - would be cool to have you helping out! 😄

martinvw · 2017-09-07T19:30:23Z

...binding.loxone/src/main/java/org/openhab/binding/loxone/handler/LoxoneMiniserverHandler.java

@@ -201,16 +211,32 @@ public void channelLinked(ChannelUID channelUID) {
    @Override
    public void initialize() {
        logger.trace("Initializing thing");
+
+        // config.as does not put restrictions on the return time (it is nullable)


Is this an error in the framework should we change it there?

PS: typo, time instead of type

martinvw · 2017-09-07T19:31:26Z

...binding.loxone/src/main/java/org/openhab/binding/loxone/handler/LoxoneMiniserverHandler.java

-                }
+            channels.addAll(newChannels);
+            for (Channel channel : newChannels) {
+                @SuppressWarnings("null")


Is this correct, or is this a bug in framework? Or does it somehow think that the channel can be null?

Channel class seems to be null by default (it has some of the fields annotated with NonNull). But uid can be null and there actually is a (package protected only) default constructor where uid is not initialized. I thing this can be fixed in the Channel class, probably by simply initializing uid to a new ChannelUUID() in the default constructor and annotating this as non null. I will remove this suppression and leave the warning, then try fix it in the framework.

martinvw · 2017-09-07T19:34:25Z

...openhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxControl.java

-        this.name = json.name;
+        String jsonName = json.name;
+        if (jsonName != null) {
+            this.name = jsonName;


Why not allow the field to be null to represent an unknown name, imho null's are not that evil, they have there place in the ecosystem (for now)

Originally I checked for this null anyway, but later, and substituted it with 'unknown'.

martinvw · 2017-09-07T19:36:00Z

...openhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxControl.java

@@ -225,17 +239,20 @@ void update(LxJsonControl json, LxContainer room, LxCategory category) {
                String value = element.getAsString();
                if (value != null) {
                    LxUuid id = new LxUuid(value);
+                    @SuppressWarnings("null")


I personally think, because of the many repeated suppressions, that this not really mature yet I see some drawback in the readability of the code.

@SuppressWarnings("null") @NonNull

It should be better when suppressions are removed from the jdk calls. The ones left will be for GSON and logger. Do you know if there is plan to fix it for the logger? This is common enough to be important.

Maybe @triller-telekom knows?

We are planning to revise our guidelines where we now state that @NonNullByDefault should be used per class. depending on the outcome of #4321 we might even disable the warnings in the IDE and only show the error cases.

Regarding third party libraries there is also PR #4217 but that one is waiting for the outcome of #4321, too.

But if you want to keep using the null annotations, please change them to the @NonNullByDefault on class level.

Seems like the whole null annotations thing went into business for itself even before we had agreed on the above mentioned guidelines. I am sorry if this now hits you so you have to change your annotations.

martinvw · 2017-09-07T19:37:43Z

...openhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxControl.java

                        logger.trace("Existing state for LxControl {}: {}", json.type, name);
+                        @SuppressWarnings("null")
+                        @NonNull
+                        LxControlState state = states.get(name);
                        state.getUuid().setUpdate(true);


Don't you have to call states.put(name, state); anymore? Or was that an error because you just restored the existing state?

Yes, this was an error, or a redundant put changing nothing, the key is already there and we don't change the object reference.

martinvw · 2017-09-25T15:36:06Z

....openhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxServer.java

+                String name = room.name;
+                if (uuid != null && name != null) {
+                    addOrUpdateRoom(new LxUuid(uuid), name);
+                }


Should we do any logging this feels like a non-happy-flow

addOrUpdateRoom is a normal flow, I could add a log to the 'else' part, which is absent now.

martinvw · 2017-09-27T20:29:24Z

addons/binding/org.openhab.binding.loxone/META-INF/MANIFEST.MF

- org.osgi.framework,
- org.slf4j
-Service-Component: OSGI-INF/*.xml
+Manifest-Version: 1.0


This is not a proper resolving of this merge conflict, please follow the new sorting / indentation etc and only add your new imports

Did you address this one?

Yes you did, only github did not show that properly :-)

martinvw · 2017-09-27T20:30:28Z

...ding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxWsStateUpdateEvent.java

 /**
 * An event received from Loxone Miniserver with control's state update
 *
 * @author Pawel Pieczul - initial contribution
 *
 */
+@NonNullByDefault


Would it be less configuration, to not use this one here?

This will result in many incompatibilities in classes that use this class and are non null by default (or we mark all method arguments in this class as non null, which is even more ugly)

Okay I did not completely realise the impact 🙂

martinvw · 2017-09-27T20:32:23Z

...rg.openhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxUuid.java

+    String cleanupUuid(String uuid) {
+        @SuppressWarnings("null")
+        @NonNull
+        String newId = uuid.replaceAll("[^a-zA-Z0-9-]", "-").toUpperCase();


Wouldn't it make more sense to annotate the method and replace the body with:

return uuid.replaceAll("[^a-zA-Z0-9-]", "-").toUpperCase();

martinvw · 2017-09-27T20:33:41Z

....openhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxServer.java

+                String type = cat.type;
+                if (uuid != null && name != null && type != null) {
+                    addOrUpdateCategory(new LxUuid(uuid), name, type);
+                }


ppieczul · 2017-09-28T13:11:42Z

@kaikreuzer @martinvw I will be honored to help, as far as my competence and time allows. Let me start with #2350 this weekend.

martinvw

@ppieczul some final remarks :-)

martinvw · 2017-10-02T07:00:37Z

...binding.loxone/src/main/java/org/openhab/binding/loxone/handler/LoxoneMiniserverHandler.java

-                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Unknown host");
+        try {
+            server = createNewServer(cfg);
+            if (server == null) {


Minor since you are already handling exceptions, maybe you might be better of not returning null from createNewServer() and throw an exception in that error scenario, then both the OFFLINE markings end up on the same level,

martinvw · 2017-10-02T07:04:57Z

...b.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/LoxoneHandlerFactory.java

 public class LoxoneHandlerFactory extends BaseThingHandlerFactory {

+    @SuppressWarnings("null")


Maybe you could consider leaving this ones out

Removing the @SuppressWarnings here will give a compiler warning....

Sets comes from com.google.common.collect package and we talked about not suppressing only JDK calls, that's why I left it. However, I am ok with removing this and leaving the warning.

That is a valid point 👍

what about dropping the unnecessary dependency to the google library and just use

public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = new HashSet<>(LoxoneBindingConstants.THING_TYPE_MINISERVER);

Probably better to use: Collections.singleton(). Which is also standard java.

martinvw · 2017-10-02T07:13:37Z

...b.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/LoxoneHandlerFactory.java

 public class LoxoneHandlerFactory extends BaseThingHandlerFactory {

+    @SuppressWarnings("null")
+    @NonNull


Please remove this

martinvw · 2017-10-02T07:14:05Z

...openhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxControl.java

            logger.trace("Reading states for LxControl: {}", json.type);

            for (Map.Entry<String, JsonElement> jsonState : json.states.entrySet()) {
+                @SuppressWarnings("null")
+                @NonNull


Please remove all @nonnull

martinvw · 2017-10-02T07:14:28Z

...penhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxJsonApp3.java

-    Map<String, LxJsonControl> controls;
-    Map<String, LxJsonRoom> rooms;
-    Map<String, LxJsonCat> cats;
+    Map<@NonNull String, @NonNull LxJsonControl> controls;


Please remove all @nonnull and use @NonNullByDefault

Then I have to add nullable to every field, since they can be null. It also gives me over 20 new warnings due to potential null access, which are these warnings for potential change of the value in multithreaded code, which will never happen here. For example:

if (a != null) { if (a.b != null) { a.b.doSomething(); } }

This will generate warning at doSomething, because a might have changed between a!=null and this line.
I think it is not worth it and would rather leave the class not annotated. But then I have to mark the Map key and value and nonnull, because they are never created null, if map is created at all and this seems to break the convention to not use nonnull.
What do you think, which is better? Maybe another solution exists, which I can't see now.

I was triggered by the remark of @triller-telekom maybe he knows more.

@triller-telekom can you enlighten us?

Maybe you want to join in eclipse-archived/smarthome#4321 with these observations/problems. The more we use those annotations, the less we are confident that it is possible to solve all the warnings in a decent way...

@kaikreuzer @ppieczul I just read the latest state of the discussion and now I'm in doubt should we just merge this as is. Or do we still want to wait for things to settle down. I think that I read that the usage of @nonnullbydefault is still the prefered way, but not all warnings have to be solved / surpressed?

I think it would be great of one of people actively involved in the discussion could review this PR, maybe @htreu @triller-telekom or @SJKA? Thanks all!

It would be great to have this reviewed by more people.

If there are generally no erroneous changes here, I think (in the spirit of continuous integration) it is more beneficial to merge this and then resolve outstanding warnings at the JDK calls - when they are solved at the framework level.

I will also summarize my findings in the smarthome disscuscion mentioned above.

With more and more nonnullbydefault added to the framework, I am now getting lots of warnings from this binding too with the version that is on master.
Also, there is a new version of Loxone server software which requires new things to be added to the binding, so it starts stacking up.
Thanks

I would vote for adding @NonNullbyDefault to the class and if your variables can all be null you would need to add the @Nullable to them.

And please add your findings regarding false positives warnings about the null annotations to our discussion in eclipse-archived/smarthome#4321. The outcome of this discussion will probably be that we change the settings within Eclipse about what we will treat as a warning/error.

For sure we should not block PRs for senseless discussions, but in this case we have the problem that we introduced the null annotations to the framework before we actually agreed on rules on how to use them :( This was a bit unfortunate because while using those annotations they turned out to be rather tricky to use and the tools (like Eclipse) do not support them perfectly. I am sorry that you had to change your PR quite some often because of this, but you were a little to quick for us ;)

martinvw · 2017-10-02T07:15:05Z

...penhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxWsClient.java

-                            if (resp != null) {
+                            try {
+                                @SuppressWarnings("null")
+                                @NonNull


Please remove all @nonnull and use @NonNullByDefault

martinvw · 2017-10-02T07:15:13Z

...penhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxWsClient.java

-                            if (resp != null) {
+                            try {
+                                @SuppressWarnings("null")
+                                @NonNull


Please remove all @nonnull and use @NonNullByDefault

martinvw · 2017-10-02T07:15:22Z

...penhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxWsClient.java

-                            if (config != null) {
+                            try {
+                                @SuppressWarnings("null")
+                                @NonNull


Please remove all @nonnull and use @NonNullByDefault

triller-telekom

Please see my comments inline in the code.

Regarding the left over warnings, please see eclipse-archived/smarthome#4321 where we are about to change the warning/error settings.

triller-telekom · 2017-10-17T08:21:31Z

...binding.loxone/src/main/java/org/openhab/binding/loxone/handler/LoxoneMiniserverHandler.java

    }

    @Override
    public void dispose() {
        logger.debug("Disposing of server");
-        if (server != null) {
-            server.stop();
+        LxServer srv = server;


Why are you copying the class variable into a local one before checking for null and stopping the server?

triller-telekom · 2017-10-17T08:30:07Z

...b.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/LoxoneHandlerFactory.java

 public class LoxoneHandlerFactory extends BaseThingHandlerFactory {

+    @SuppressWarnings("null")


what about dropping the unnecessary dependency to the google library and just use

public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = new HashSet<>(LoxoneBindingConstants.THING_TYPE_MINISERVER);

triller-telekom · 2017-10-17T08:31:51Z

...openhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxControl.java

@@ -102,11 +139,13 @@ public LxControlState getState(String name) {
     * Call when control is no more needed - unlink it from containers
     */
    public void dispose() {
-        if (room != null) {
-            room.removeControl(this);
+        LxContainer localRoom = room;


Same as above: Why do you copy the class variable into a local one?

triller-telekom · 2017-10-17T08:31:56Z

...openhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxControl.java

        }
-        if (category != null) {
-            category.removeControl(this);
+        LxCategory localCat = category;


Same as above: Why do you copy the class variable into a local one?

triller-telekom · 2017-10-17T08:42:58Z

...penhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxJsonApp3.java

-    Map<String, LxJsonControl> controls;
-    Map<String, LxJsonRoom> rooms;
-    Map<String, LxJsonCat> cats;
+    Map<@NonNull String, @NonNull LxJsonControl> controls;


I would vote for adding @NonNullbyDefault to the class and if your variables can all be null you would need to add the @Nullable to them.

And please add your findings regarding false positives warnings about the null annotations to our discussion in eclipse-archived/smarthome#4321. The outcome of this discussion will probably be that we change the settings within Eclipse about what we will treat as a warning/error.

For sure we should not block PRs for senseless discussions, but in this case we have the problem that we introduced the null annotations to the framework before we actually agreed on rules on how to use them :( This was a bit unfortunate because while using those annotations they turned out to be rather tricky to use and the tools (like Eclipse) do not support them perfectly. I am sorry that you had to change your PR quite some often because of this, but you were a little to quick for us ;)

triller-telekom · 2017-10-17T08:46:02Z

...penhab.binding.loxone/src/main/java/org/openhab/binding/loxone/internal/core/LxWsClient.java

@@ -255,22 +260,22 @@ boolean connect() {

        synchronized (state) {
            socket = new LxWebSocket();
-            wsClient = new WebSocketClient();
+            WebSocketClient client = new WebSocketClient();
+            wsClient = client;


Why do you create a local variable at all?

This is to avoid a warning about potential null access, because theoretically the indirect reference may have changed between the check and use.

‘’’
if (a.b != null) {
x(a.b)
y(a.b)
}
‘’’

Apparently in constructions like that compiler will not optimize to cache the referenced value, because this is not what we ask for coding it like that. If the code flow allows for preemption to another thread, the subsequent use of a.b may find b as null. This is my explanation as to why there is a warning.
This leads to awkward constructions with local variables. I would like to eliminate that and this may lead to warning level tuning but I don’t remember now which one was responsible for that.

If you just introduced the local variable to satisfy the warnings of the eclipse nullchecker, please undo your changes. We definitely do not want to code around those issue.

But as it is within a synchronized block I am assuming that wsClient can indeed be changed by another thread, is that correct? If not, please undo your changes as I mentioned above.

Set NonNullByDefault on all packages except thing factory. Removed dead code and redundant null checks, added missing null checks. Added thread-safe constructions with local object references to eliminate possible null access warnings. Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

…trolDimmer. Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

…simplify the code of control classes. Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

The warnings left are related to JDK calls that show nullness incompatibilities. Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

…by default to JSON deserialization classes. Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

ppieczul · 2017-10-22T13:51:16Z

I removed the extra variable as @triller-telekom suggested and also made the JSON deserialization classes non null by default.
However, I had to add the variables back in a few cases where mvn reported error (but IDE just warning). I don't really know the difference between these cases and those that only cause warnings. You can see these causing errors in the last commit.

ppieczul · 2017-10-29T20:25:43Z

I am putting this PR on hold. I need merging of #2786 first. This will be followed by implementation of new controls and authentication mechanism that were introduced to the most recent version of Loxone Miniserver software. These are more important than null annotations. I will reopen this PR once the other PRs are done.

ppieczul force-pushed the loxone-3 branch from 02c1296 to e641100 Compare September 24, 2017 12:26

ppieczul mentioned this pull request Sep 27, 2017

[Loxone] Adding dimmer and timed switch functionality #2667

Merged

ppieczul force-pushed the loxone-3 branch from 6da6db7 to 1564bf3 Compare September 27, 2017 16:50

martinvw reviewed Sep 27, 2017

View reviewed changes

martinvw mentioned this pull request Sep 27, 2017

Renamed a few packages in the loxone binding. #2741

Closed

martinvw reviewed Oct 2, 2017

View reviewed changes

ppieczul force-pushed the loxone-3 branch from 2b18c91 to 589562f Compare October 6, 2017 15:51

ppieczul mentioned this pull request Oct 8, 2017

LG webOS binding #2768

Merged

triller-telekom reviewed Oct 17, 2017

View reviewed changes

ppieczul added 7 commits October 22, 2017 14:36

Move internal packages to internal

5baa04e

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

Added DS annotations, removed xmls and added .gitignore

9193413

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

Added non null by default to the handler factory

1d48260

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

Fixed static analysis formatting guidelines

37d73bd

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

NonNullByDefault moved from package-info to each class

d347a2e

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

Added null annotation to new controls: LxControlTimedSwitch and LxCon…

f85dd48

…trolDimmer. Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

ppieczul added 4 commits October 22, 2017 14:36

Refactoring: added methods to get control's state value directly, to …

3b2e749

…simplify the code of control classes. Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

Implemented code review comments.

a975fc2

The warnings left are related to JDK calls that show nullness incompatibilities. Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

More review comments implemented.

bd412df

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

Reverted additinal variables that prevented warnings. Added non null …

9cfa5c7

…by default to JSON deserialization classes. Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

ppieczul force-pushed the loxone-3 branch from 589562f to 9cfa5c7 Compare October 22, 2017 13:27

Added extra variables where mvn reported errors.

97642bd

Signed-off-by: Pawel Pieczul <pieczul@gmail.com> (github: ppieczul)

ppieczul closed this Oct 29, 2017

ppieczul mentioned this pull request Oct 30, 2017

[Loxone] Added dynamic channel state description provider #2786

Merged

		public class LoxoneHandlerFactory extends BaseThingHandlerFactory {

		@SuppressWarnings("null")

[Loxone] Added null annotations #2604

[Loxone] Added null annotations #2604

Conversation

ppieczul commented Aug 27, 2017

ppieczul commented Sep 2, 2017

ppieczul commented Sep 9, 2017

kaikreuzer commented Sep 11, 2017

martinvw commented Sep 22, 2017

ppieczul commented Sep 22, 2017 via email

ppieczul commented Sep 23, 2017

ppieczul commented Sep 24, 2017

maggu2810 commented Sep 24, 2017

ppieczul commented Sep 24, 2017 • edited Loading

ppieczul commented Sep 25, 2017

ppieczul commented Sep 27, 2017

martinvw commented Sep 27, 2017

kaikreuzer commented Sep 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

triller-telekom Sep 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppieczul commented Sep 28, 2017

martinvw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

triller-telekom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppieczul commented Oct 22, 2017

ppieczul commented Oct 29, 2017

ppieczul commented Sep 24, 2017 •

edited

Loading

triller-telekom Sep 29, 2017 •

edited

Loading