[miio] add miot protocol & conditions #7404

marcelrv · 2020-04-17T20:53:38Z

Add miot protocol handling [miio] Binding enhancements #7276
Add first 2 miot devices (airpurifiers mb3 & ma4)
Fix multiple command handling
Add conditional commands
Change brightness to dimmers closing [miio] Brightness Channel as Dimmer #4388
Apply conditions for dimmers https://community.openhab.org/t/xiaomi-phillips-zhirui-with-alexa-how-can-i-combine-power-and-dimming/59251/4
Remove obsolete (pre)parameters from actions

Signed-off-by: Marcel Verpaalen marcel@verpaalen.com

* Add miot protocol handling openhab#7276 * Add first 2 miot devices (airpurifiers mb3 & ma4) * Add conditional commands * Change brightness to dimmers openhab#4388 * Apply conditions for dimmers * Remove obsolete (pre)parameters from actions Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>

TravisBuddy · 2020-04-17T20:56:20Z

Travis tests were successful

Hey @marcelrv,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

TravisBuddy · 2020-04-17T21:37:56Z

Travis tests were successful

Hey @marcelrv,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

cpmeister · 2020-04-17T22:23:17Z

...hab.binding.miio/src/main/java/org/openhab/binding/miio/internal/basic/ActionConditions.java

+     */
+    private static @Nullable JsonElement firmwareCheck(MiIoDeviceActionCondition condition,
+            Map<String, Object> deviceVariables, @Nullable JsonElement value) {
+        // TODO: placeholder for firmware version check to allow for firmware dependent actions


Is this a todo or a comment?

It indeed is a placeholder for now for something I want to implement in a further version.
It does require still bit of researching on the format of the various firmware version strings Xiaomi is using.

if it is disturbing I can remove the whole function until it is actually used.

You can leave this as is.

cpmeister · 2020-04-17T22:23:33Z

...hab.binding.miio/src/main/java/org/openhab/binding/miio/internal/basic/ActionConditions.java

+     * @param condition
+     * @param deviceVariables
+     * @param value
+     * @return value is case firmware is matching, return null if not


Suggested change

* @return value is case firmware is matching, return null if not

* @return value in case firmware is matching, return null if not

cpmeister · 2020-04-17T22:24:26Z

...hab.binding.miio/src/main/java/org/openhab/binding/miio/internal/basic/ActionConditions.java

+                if (intVal > 0 && intVal < 99) {
+                    return value;
+                }
+            } catch (ClassCastException e) {


Can you check for this before trying to cast it rather than catching the exception?

yes, didn't find it at first, but found a number check.
done

cpmeister · 2020-04-17T22:25:34Z

...hab.binding.miio/src/main/java/org/openhab/binding/miio/internal/basic/MiIoBasicChannel.java

+        }
+    }
+
+    public void setPiid(Integer piid) {


If this is non-null by default why not make this a primitive?

These siid & piid Integers are only used for miot protocol devices, I use it to determine if it is a miot device by comparing to null in the isMiot function.

in the further functions I don't want to do this checking anymore hence the conversion here to primitive

cpmeister · 2020-04-17T22:37:00Z

...inding.miio/src/main/java/org/openhab/binding/miio/internal/handler/MiIoAbstractHandler.java

        }
        updateProperties(properties);


Why not just do:

Suggested change

}

updateProperties(properties);

}

deviceVariables.putAll(properties);

updateProperties(properties);

Excellent suggestion...done

Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>

TravisBuddy · 2020-04-18T08:24:01Z

Travis tests were successful

Hey @marcelrv,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

marcelrv · 2020-04-18T11:58:16Z

thanks @cpmeister made the suggested updates.

cpmeister · 2020-04-18T15:27:52Z

...b.binding.miio/src/main/java/org/openhab/binding/miio/internal/handler/MiIoBasicHandler.java

-            String param = para.get(i).getAsString();
+            // This is a miot parameter
+            String param;
+            if (para.get(i).isJsonObject()) { // miot channel


You should cache para.get(i) in a local variable for reuse.

cpmeister · 2020-04-18T15:31:02Z

...b.binding.miio/src/main/java/org/openhab/binding/miio/internal/handler/MiIoBasicHandler.java

+                            Color color = Color.getHSBColor(hsb.getHue().floatValue() / 360,
+                                    hsb.getSaturation().floatValue() / 100, hsb.getBrightness().floatValue() / 100);
+                            value = new JsonPrimitive(
+                                    (color.getRed() * 65536) + (color.getGreen() * 256) + color.getBlue());


This calculation would look better using bit-shift operators instead of multiplication.

Suggested change

(color.getRed() * 65536) + (color.getGreen() * 256) + color.getBlue());

(color.getRed() << 16) + (color.getGreen() << 8) + color.getBlue());

Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>

TravisBuddy · 2020-04-18T16:07:46Z

Travis tests were successful

Hey @marcelrv,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

cpmeister · 2020-04-18T16:10:00Z

Manually checked sign-off

* [miio] add miot protocol & conditions * Add miot protocol handling openhab#7276 * Add first 2 miot devices (airpurifiers mb3 & ma4) * Add conditional commands * Change brightness to dimmers openhab#4388 * Apply conditions for dimmers * Remove obsolete (pre)parameters from actions Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com> Signed-off-by: Eugen Freiter <freiter@gmx.de>

* [miio] add miot protocol & conditions * Add miot protocol handling openhab#7276 * Add first 2 miot devices (airpurifiers mb3 & ma4) * Add conditional commands * Change brightness to dimmers openhab#4388 * Apply conditions for dimmers * Remove obsolete (pre)parameters from actions Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>

* [miio] add miot protocol & conditions * Add miot protocol handling openhab#7276 * Add first 2 miot devices (airpurifiers mb3 & ma4) * Add conditional commands * Change brightness to dimmers openhab#4388 * Apply conditions for dimmers * Remove obsolete (pre)parameters from actions Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com> Signed-off-by: CSchlipp <christian@schlipp.de>

* [miio] add miot protocol & conditions * Add miot protocol handling openhab#7276 * Add first 2 miot devices (airpurifiers mb3 & ma4) * Add conditional commands * Change brightness to dimmers openhab#4388 * Apply conditions for dimmers * Remove obsolete (pre)parameters from actions Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>

* [miio] add miot protocol & conditions * Add miot protocol handling openhab#7276 * Add first 2 miot devices (airpurifiers mb3 & ma4) * Add conditional commands * Change brightness to dimmers openhab#4388 * Apply conditions for dimmers * Remove obsolete (pre)parameters from actions Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com> Signed-off-by: Daan Meijer <daan@studioseptember.nl>

* [miio] add miot protocol & conditions * Add miot protocol handling openhab#7276 * Add first 2 miot devices (airpurifiers mb3 & ma4) * Add conditional commands * Change brightness to dimmers openhab#4388 * Apply conditions for dimmers * Remove obsolete (pre)parameters from actions Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>

marcelrv requested a review from cpmeister April 17, 2020 20:53

marcelrv linked an issue Apr 17, 2020 that may be closed by this pull request

[miio] Brightness Channel as Dimmer #4388

Closed

marcelrv added the enhancement An enhancement or new feature for an existing add-on label Apr 17, 2020

[miio] remove DTO files not relevant to this PR

0fc6173

cpmeister requested changes Apr 18, 2020

View reviewed changes

[miio] updates from code review

c39d789

Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>

cpmeister requested changes Apr 18, 2020

View reviewed changes

[miio] updates from code review

c479de2

Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>

cpmeister merged commit 94b422e into openhab:2.5.x Apr 18, 2020

cpmeister added this to the 2.5.4 milestone Apr 18, 2020

marcelrv deleted the miio-miot branch April 18, 2020 18:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[miio] add miot protocol & conditions #7404

[miio] add miot protocol & conditions #7404

marcelrv commented Apr 17, 2020 •

edited

Loading

TravisBuddy commented Apr 17, 2020

TravisBuddy commented Apr 17, 2020

cpmeister Apr 17, 2020

marcelrv Apr 18, 2020

cpmeister Apr 18, 2020

cpmeister Apr 17, 2020

marcelrv Apr 18, 2020

cpmeister Apr 17, 2020

marcelrv Apr 18, 2020

cpmeister Apr 17, 2020

marcelrv Apr 18, 2020

cpmeister Apr 17, 2020

marcelrv Apr 18, 2020

TravisBuddy commented Apr 18, 2020

marcelrv commented Apr 18, 2020

cpmeister Apr 18, 2020

cpmeister Apr 18, 2020

TravisBuddy commented Apr 18, 2020

cpmeister commented Apr 18, 2020

	* @return value is case firmware is matching, return null if not
	* @return value in case firmware is matching, return null if not

	(color.getRed() * 65536) + (color.getGreen() * 256) + color.getBlue());
	(color.getRed() << 16) + (color.getGreen() << 8) + color.getBlue());

[miio] add miot protocol & conditions #7404

[miio] add miot protocol & conditions #7404

Conversation

marcelrv commented Apr 17, 2020 • edited Loading

TravisBuddy commented Apr 17, 2020

Travis tests were successful

TravisBuddy commented Apr 17, 2020

Travis tests were successful

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

TravisBuddy commented Apr 18, 2020

Travis tests were successful

marcelrv commented Apr 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TravisBuddy commented Apr 18, 2020

Travis tests were successful

cpmeister commented Apr 18, 2020

marcelrv commented Apr 17, 2020 •

edited

Loading