SANdood / Aeon-HEM-v2

My version of the ST device driver for the Aeon HEM, exended to support the V2 version, with more displays
42 stars 121 forks source link

Fixing to support Android #7

Closed KyleLeneau closed 6 years ago

KyleLeneau commented 8 years ago

Hello,

Mobile developer/manager at SmartThings here (Kyle). I had a co-worker install this and report an issue with Android. I copied your code, installed it and sourced an issue that you should make to support Android. On the value tiles you are specifying value and color ranges for the tile but the value you are using is not a valid Double. You need to remove the units from those. Example:

Before (background colors on a value tile): [value: "0 Watts", color: "#153591"], [value: "3000 Watts", color: "#1e9cbb"]

After: [value: "0", color: "#153591"], [value: "3000", color: "#1e9cbb"]

SANdood commented 8 years ago

Thanks.

Works on IOS - mebbe SmartThings should fix their Android implementation.

  Barry

Sent from my iPhone

On Dec 31, 2015, at 9:08 PM, Kyle LeNeau notifications@github.com wrote:

Hello,

Mobile developer/manager at SmartThings here (Kyle). I had a co-worker install this and report an issue with Android. I copied your code, installed it and sourced an issue that you should make to support Android. On the value tiles you are specifying value and color ranges for the tile but the value you are using is not a valid Double. You need to remove the units from those. Example:

Before (background colors on a value tile): [value: "0 Watts", color: "#153591"], [value: "3000 Watts", color: "#1e9cbb"]

After: [value: "0", color: "#153591"], [value: "3000", color: "#1e9cbb"]

You can view, comment on, or merge this pull request online at:

https://github.com/SANdood/Aeon-HEM-v2/pull/7

Commit Summary

Fixing to support Android File Changes

M Aeon HEMv2.groovy (160) Patch Links:

https://github.com/SANdood/Aeon-HEM-v2/pull/7.patch https://github.com/SANdood/Aeon-HEM-v2/pull/7.diff — Reply to this email directly or view it on GitHub.

KyleLeneau commented 8 years ago

The Android implementation is actually more right then iOS because it enforces the type that the tile expect according to the docs. Looks like a place where we need to update the documentation for.

backgroundColors List - Specify a list of maps of attribute values and colors. The mobile app will match and interpolate between these entries to select a color based on the value of the attribute.

SANdood commented 8 years ago

Yeah - no offense. But iOS has been this way for more than a year, so asking me to adjust to a broken/incomplete-but-more-correct Android implementation is pretty lame. More appropriate that ST make the Android implementation match that of iOS than to force a regression on the iOS user community.

Especially since the "not right" iOS implementation enables a more rich user experience (color+custom display text).

So, will you seriously recommend breaking the current iOS implementation to match the (limited) Android app?

  Barry

Sent from my iPhone

On Dec 31, 2015, at 9:22 PM, Kyle LeNeau notifications@github.com wrote:

The Android implementation is actually more right then iOS because it enforces the type that the tile expect according to the docs. Looks like a place where we need to update the documentation for.

backgroundColors List - Specify a list of maps of attribute values and colors. The mobile app will match and interpolate between these entries to select a color based on the value of the attribute.

— Reply to this email directly or view it on GitHub.

KyleLeneau commented 8 years ago

You can achieve custom label AND custom background colors. Check out this commit to my PR which adds back the custom labels: https://github.com/KyleLeneau/Aeon-HEM-v2/commit/eadedb15fc788a4d02275cea34e0f16415fa0bba

Check out the commit, you will actually see that it's the exact same functionality exists now (like you originally had it) in both iOS, Android and Windows this way.

SANdood commented 8 years ago

Yes, well your "fix" only works for 1 of the two displays - when you toggle, the same widgets (PowerOne, PowerTwo, etc.) are used to display the TIME of the highest/lowest value instead of "Watts". Further, there is a request to change to "KW" when the value exceeds 1000 Watts...

Like I said, the iOS implementation affords a richer user interface - you Android guys should really just adopt the same code instead of fighting for what you think is more "right". You already have caused THOUSANDS of users who try to use my device on Android to learn the hard way that it doesn't work without edits. A little of your time to match IOS would save a lot of grief and aggravation amongst the ST user community...pretty important, by the way, given all the other crap that we/they have had to tolerate over the past year (outages, missed timers, frozen hubs thanks to bad firmware updates, etc.).

Customers first will get you a LOT further than "we know better..."

Trust me.

vlaminck commented 6 years ago

@SANdood can you merge this? It's causing a lot of errors.