HomeSeer / Plugin-SDK

Plugin development kit for the 4th major edition of the HomeSeer platform.
https://www.nuget.org/packages/HomeSeer-PluginSDK/
GNU Affero General Public License v3.0
21 stars 4 forks source link

Due to a lack of ValueRange.Divisor control cannot be created at this time #281

Closed rmasonjr closed 1 year ago

rmasonjr commented 2 years ago

Environment

HomeSeer

OS

[Windows, Linux, etc]

HS Version

[v4.2.9.0]

Development

PSDK Version

[v1.4.0.0]

Language

[VB or C#]

IDE

[VS2017, VS2019, JetBrains Rider, etc]

Dev OS

[Windows, Linux, Mac]

Plugin

What plugin is this issue for?

Problem

Description

Describe the issue you are encountering. What happened?

Expected Behavior

What did you expect to happen?

Steps to Reproduce

Provide steps so the HomeSeer team can reproduce the reported problem:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Screenshots

If applicable, add screenshots to help explain your problem.

Logs

If applicable, include log output from the plugin console and/or the HomeSeer logs. If the logs take up more than a few lines, consider attaching a file.

rmasonjr commented 2 years ago

I'm using the new thermostat example plugin, and I'm getting this message:

Dim range = New ValueRange(0, 100) With { .DecimalPlaces = 1, .Suffix = " %" } ff.AddValueDropDown(range, New ControlLocation(1, 1), EControlUse.NotSpecified)

Public Overloads Function AddValueDropDown(targetRange As ValueRange, [location As ControlLocation = Nothing], [controlUse As EControlUse = NotSpecified]) As FeatureFactory' is obsolete: 'Due to a lack of ValueRange.Divisor property this control type cannot be properly created at this time.'.

Thanks Rob

spudwebb commented 2 years ago

A Divisor property or something similar is planned to be added soon, but the message you see is just a compilation warning, for a range of 0-100 your code should just work fine as is.

rmasonjr commented 2 years ago

So, how do we make a dropdown list with celsius values? In the code above, I tell it 1 decimal place, but the dropdown list has whole integers (with .0) A celsius dropdown list should have 25.0, 25.5, 26.0, 26.5, etc.

spudwebb commented 2 years ago

To display half degrees in the list we need to implement the divisor property. Tracked as PSDK-112

sjhill01 commented 1 year ago

My thermostat customers using Celsius are also held up by the lack of this property. +1 begging for it.

sjhill01 commented 1 year ago

This is going to require more than just PSDK code I assume, since at a minimum the html will need to be updated for the feature status control options dialog. I can submit a pull with the divisor property and the value range GetStringForValue method, plus the associated test cases with a non-1 divisor, but I don't see that method being called when generating the dropdown list so I'm not sure how helpful that is at this point.

rjhelmke commented 1 year ago

I have a build you can test, please report if this is working ok for you. You can set the Divisor on the range object like: ValueRange range = new ValueRange(1, 100); range.DecimalPlaces = 1; range.Divisor = 10; ff.AddValueDropDown(range);

Here is the test build for Windows: https://homeseer.com/updates4/SetupHS4_4_2_18_10.msi

Many thanks to sjhill01 for the pull request.

rmasonjr commented 1 year ago

Thanks Rich - the PluginSDK.dll in that install works fine:

image

image

spudwebb commented 1 year ago

@rmasonjr why does the current value is displayed as 74.0 ? Shouldn't it be 37 with a " C" suffix?

rmasonjr commented 1 year ago

@spudwebb oh wow - good catch.

I'm looking at the html code that was built for the dropdown and it contains the F values - maybe @rjhelmke can confirm?

Here's the html code that the PluginSDK generated:

image

image

rjhelmke commented 1 year ago

@rmasonjr @spudwebb You actually set the value on the device from your plugin so I would say that you should take the value submitted and then divide it by your divisor and set the value. We could do this in HS but I am not sure of the ramifications. I checked this in the HS3 UI and it works the same.

spudwebb commented 1 year ago

@rmasonjr you have to define your StatusGraphic range with the same range options than the control range. You can't do that using AddGraphicForRange, so use AddGraphic() instead

                var sg = new StatusGraphic(IMAGES_STATUS_DIR + "Thermometer-00.png", -200, 200);
                sg.TargetRange.DecimalPlaces = useCelsius ? 1 : 0;
                sg.TargetRange.Suffix = useCelsius ? " °C" : " °F";
                sg.TargetRange.Divisor = useCelsius ? 2 : 1;
                ff.AddGraphic(sg);

Also if you're not using the divisor (F case) it needs to be set to 1, not 0

Remember that the divisor will be used to display any value of this feature in the UI. So for example if you set the value of this temp feature to 50 using UpdateFeatureValueByRef(), the real value of the feature will be set to 50, but the displayed value will be set to "25 °C"

we are going to update the sample plugin thermostats to show how to use that new Divisor property:

image

spudwebb commented 1 year ago

@rmasonjr @spudwebb You actually set the value on the device from your plugin so I would say that you should take the value submitted and then divide it by your divisor and set the value. We could do this in HS but I am not sure of the ramifications. I checked this in the HS3 UI and it works the same.

The value submitted is actually the non-divided value, i.e if you select 37 from the dropdown list, the value received in SetIOMulti is 74, and the plugin should set this same value 74 using UpdateFeatureValueByRef to get 37 as the current value, (assuming the status graphic range uses the same divisor as explained in my previous post)

rmasonjr commented 1 year ago

Holy cow - this is really confusing. I dont remember having to manually divide a value by a divisor in the HS3 plugin.

When a thermostat setpoint is set at the unit, my plugin would receive 23.5 C, I would set the value to 23.5 and the status text would be 23.5 °C

When a thermostat setpoint is set in the HS device page, the user would select 24.0 from the dropdown, I would send the value 24.0 to the OMNI, then the status text would be 24 °C

In your example, why would the 37 selection from the dropdown send 74? Why not just send 37?

All I am trying to do is get .5 precision in the dropdown.

rmasonjr commented 1 year ago

@spudwebb @rjhelmke - any ideas on getting .5 precision for the dropdown?

rjhelmke commented 1 year ago

I thought the build I posted for you was working ok? The only issue I saw you post was that the value posted from the dropdown was twice the value viewed. So you just need to divide that by your divisor to get the actual value that was selected. Am I missing something?

rmasonjr commented 1 year ago

I'm not understanding why I need to divide - never had to do that with an HS3 thermostat setpoint.

Maybe Divisor is not what I'm looking for.

I need a dropdown with celsius values in .5 precision for both the selection and the value in the HTML. So, when the user selects 37.5, from the dropdown, I send 37.5 to the thermostat. I dont want to take 74 and divide it by a divisor and send it. A dropdown should just send the real value of the selection.

Am I not using the ValueRange object correctly? Here's my code:

 Dim vr = New ValueRange(-39, 122) With {
                .DecimalPlaces = IIf(useCelsius, 1, 0),
                .Suffix = IIf(useCelsius, " °C", " °F"),
                .Divisor = IIf(useCelsius, 2, 1)
 }
rjhelmke commented 1 year ago

I was trying to get a fix for you so you could get your plugin out. I understand it is a bit confusing and we plan on making some changes that will include a divisor and a step property. But we need to do this carefully and it will take a little time. For now just divide the returned result by your divisor and set that as the status. We will let you know when we have an updated solution. Note that HS3 had the same issue and you can go to the old /deviceutility page and see the same results. An alternate solution for you is to add +/- buttons and not use a droplist. You have total control over the step in that case.

rmasonjr commented 1 year ago

Please make a step property. As it stands now, ValueRange is not useful for a celsius control/value. Even the sample plugin is wrong - BuildSetpointFeature is just building a dropdown with whole numbers. The increment/decrement controls do allow you to change it, but the dropdown does not contain those .5 values.

I'm going to make it a textbox for now instead of a dropdown. I hate doing that, but there's not a better way. Dividing by the divisor just seems crazy to me.

Thanks Rob