SelectListViews return from HS4 only contain changes

[skiingwiz]

Environment and System Config

• OS: Windows (plugin), Linux (HS4)
• Language: C#
• IDE: VS2019
• HomeSeer Version: v4.1.8.0
• PluginSDK Version: v1.2.1

Describe the issue When using a SelectListView from the Page object passed to the OnSettingPageSave method, the GetSelectedOption and GetSelectedOptionKey methods are not behaving as expected. GetSelectedOption is returning the Selection value (as a string). GetSelectedOptionKey is returning nothing.

Steps to Reproduce Steps to reproduce the behavior:

1. Create a settings page using PageFactory including the WithDropDownSelectList(string, string, List<string>, List<string>) method.
2. Override the void OnSettingPageSave(Page pageDelta) method
3. In that method loop over the pageDelta.Views
4. When you find the SelectListView, call the GetSelectedOption and GetSelectedOptionKey method
5. Observe that the values returned are described above, rather than what's expected (below)

Expected behavior GetSelectedOption should return the value from the options list passed to PageFactory.WithDropDownSelectList for the item selected. GetSelectedOptionKey should return the value from the optionKeys list passed to PageFactory.WithDropDownSelectList for the item selected.

---

Additional Info This issue can be worked around by using the index returned by Selection to retrieve the values from the original lists passed to WithDropDownSelectList.

[mnsandler]

using SDK 1.2.1 within ActionAbstractType class GetSelectedOptionKey doesn't provide the currently value in OnConfigItemUpdate. it provides the previous value

[jldubz]

@skiingwiz ,

This is by design. There is no need to pass the options back through JSON since, as you pointed out in your additional info,

This issue can be worked around by using the index returned by Selection to retrieve the values from the original lists...

It was also left out because piecing the select list data back together from HTML is not always straight forward and can be expensive to perform client-side.

Are you building the page every time OnSettingsPageSave() is being called? Is there a reason you can't build it once during Init() and then cache it for later? I recommend this approach. This way, you can use the SelectListView.Selection returned from the client to query the cached select list.

For example Given the following page:

//Start a PageFactory to construct the Page
var settingsPage1 = PageFactory.CreateSettingsPage("settings-page1", "Sample Settings");
var options = new List<string> 
{
    "Option 1",
    "Option 2",
    "Option 3"
};
//Add a drop down select list to the page
settingsPage1.WithDropDownSelectList("settings-page1-selectlist1",  "Sample Dropdown", options);

and assuming we receive a change in AbstractPlugin.OnSettingsPageSave(), which will by default call UpdateViewById() on the changed view if OnSettingChange() returns true. This will end up calling SelectListView.Update() passing in the partial select list (the delta communicated by the client). This will update SelectListView.Selection while retaining the existing options and keys. So the call stack will look something like this:

AbstractPlugin.OnSettingsPageSave(Page)
settingsPage1.UpdateViewById(AbstractView viewDelta)
ViewCollectionHelper.UpdateViewById(AbstractView viewDelta, ref List<AbstractView>, ref Dictionary<string, int>)
AbstractView.Update(AbstractView viewDelta)
SelectListView.Update(AbstractView viewDelta)
this.Selection = viewDelta.Selection

@mnsandler I recommend the same approach for actions and triggers. The entire Page should be stored in the data attached to the action/trigger. This contains the options and keys stored in select lists while the data passed into OnConfigItemUpdate() is merely the delta.

[jldubz]

This is also brought up in https://github.com/HomeSeer/Plugin-SDK/issues/201 and https://github.com/HomeSeer/Plugin-SDK/issues/223. I am merging all of these into this issue.

This issue pertains to the behavior of the SelectListView for events and settings. The SelectListView sent back from a change event on HS4 will not contain the original values.

Tracked by PSDK-101 (internal HS ticket)

[spudwebb]

@alexbk66, @mnsandler, @dpmurphy, @skiingwiz we are fixing this issue in PluginSdk version 1.4.3 , HS4 version 4.2.17.0 I want to make sure we are not breaking existing plugins:

So, before the change (HS4 version 4.2.16 and before) if you had a page defined like this

//Start a PageFactory to construct the Page
var settingsPage1 = PageFactory.CreateSettingsPage("settings-page1", "Sample Settings");
var options = new List<string> 
{
    "Option 1",
    "Option 2",
    "Option 3"
};
var optionKeys = new List<string> 
{
    "k1",
    "k2",
    "k3"
};
//Add a drop down select list to the page
settingsPage1.WithDropDownSelectList("settings-page1-selectlist1",  "Sample Dropdown", options, optionKeys);

and if user selected Option2 and saved, then in the OnSettingPageSave method the plugin would receive a SelectListView with the following behavior:

• Selection property set to 1
• GetSelectedOption() returns "1"
• GetSelectedOptionKey() returns ""

now in HS4 version 4.2.17.0, the SelectListView received in OnSettingPageSave will have the following behavior:

• Selection property set to 1
• GetSelectedOption() returns "Option 2"
• GetSelectedOptionKey() returns "k2"

Basically Selection property stay the same but the GetSelectedOption and GetSelectedOptionKey methods will now return what is expected. So please check your existing plugin code and make sure it use the Selection property rather than the GetSelectedOption() to know which option is selected in OnSettingPageSave for settings page, in OnDeviceConfigChange for device config pages, and OnConfigItemUpdate for actions and triggers.

[mnsandler]

@spudwebb is there a beta version we can test before the release? I dont want this to hit during the holidays untested

[alexbk66]

@spudwebb thank you, it's much more convenient. But as always, this change will break existing plugins after users update HS to version 4.1.17.0? I can't imagine how to synchronise my plugin changes (and I have 16 HS4 plugins) with the users updating HS?

You shouldn't really change the existing API, even if it's ugly. You should add a new method, i.e. GetSelectedOptionEx()

[spudwebb]

@alexbk66 , I agree in most cases, but here GetSelectedOption() has never returned the expected result for an updated view, so I don't see why developers would use GetSelectedOption() rather than the Selection property, and that's what I am trying to confirm here. Do you use GetSelectedOption() when receiving an updated view in in OnSettingPageSave for settings page, in OnDeviceConfigChange for device config pages, and OnConfigItemUpdate for actions and triggers?

[mnsandler]

@spudwebb; looks like Im calling both GetSelectedOption and GetSelectedOptionKey in my own wrapper function.

I would like you to hold off releasing this change until the new year. I don't have time to test changes at this point.

[alexbk66]

@spudwebb I also use GetSelectedOption() and GetSelectedOptionKey() in my PluginLib, which I use in my all HS4 plugins (16).

I'm not sure now how and why I use it - I created PluginLib 2 years ago and need time to investigate now how it works...

[spudwebb]

Maybe I wasn't clear, in most cases the behavior of GetSelectedOption() and GetSelectedOptionKey() has not changed, they were already working as expected. It is only in the specific case where HS sends an updated view for the plugin to process that those methods now return the expected value.

For example if you take my code example above, when the user save the settings page then OnSettingPageSave(Page pageDelta) is called in your plugin.

• With HS 4.2,16,0 the pageDelta contains a SelectListView where only the Id and the Selection value are populated, all the other fields are not populated like OptionKeys, or populated with dummy values like Options
• With HS 4.2.17.0 the pageDelta now contains a SelectListView fully populated, which result in GetSelectedOption and GetSelectedOptionKey now returning expected values.

@alexbk66 , I doubt you use this processSelectListViewHtml method on an unpopulated SelectListView, as it would not work at all.

What you should look at is what you do when you receive a SelectListView in OnSettingPageSave. You can post your code here so that I can check if this will be a problem or not.

I will also send you the beta version to test as soon as it is ready.

[mnsandler]

i dont see any calls to of GetSelectedOption() and GetSelectedOptionKey() from my OnSettingPageSave method

I'm going to warn my users about upgrading to .17. i'm not making any code changes right now

[spudwebb]

Could you test build 4.2.16.4 which includes the change: https://homeseer.com/updates4/SetupHS4_4_2_16_4.msi

[mnsandler]

i loaded 4.2.16.4 and gave it quick test from the one of my Settings pages. Seems to be working fine.

I do not call either GetSelectedOption and GetSelectedOptionKey in OnSettingPageSave

fwiw, i seem to be using GetStringValue to get the new value

[spudwebb]

Thanks for testing. You should also test your event actions and triggers and device config tab if you have any.

[stefxx]

Dangerous change!

But I have searched all my plugins and no occurrence of GetSelectedOption or GetSelectedOptionKey was found.

[mnsandler]

I use both GetSelectedOption or GetSelectedOptionKey in my triggers and actions. I don't have time to do exhaustive testing until Jan.

I will post a notice to my forum just in case users upgrade and have issues.

[stefxx]

Do we have a planned release date for 4.2.17.0?

[stefxx]

It seems more has changed than just GetSelectedOption or GetSelectedOptionKey.

Using 4.2.16.4 beta is giving an issue with several of my plugins. I don't really have time to invest it right now (getting ready for Christmas Eve!), but this code:

Dim MessageTypeSelected = TryCast(ConfigPage.GetViewById($"{PageId}-msgtype"), SelectListView) MessageType = MessageTypeSelected.OptionKeys(MessageTypeSelected.Selection)

Is now crashing with a "System.ArgumentOutOfRangeException: 'Index was out of range". It seems that with the Pluginsdk.dll 1.5 (part of 4.2.16.4 beta), MessageTypeSelected.Selection always returns -1. Before, it returns the correct selected option.

Please reconsider this breaking change.

[stefxx]

I found the issue, nothing to do with the above. It seems that PluginSDK 1.5 has another change...

configViewChange (in OnConfigItemUpdate) has 2 properties, .Id and .Name, which where always equal. But since PluginSDK 1.5 the .Name suddenly contains the actual name! I guess it was always supposed to have the name which is now fixed. But I used the .Name property for some comparison which now fails. Using .Id instead fixes the problem.

I guess I have to update a few of my plugins...

[spudwebb]

You're right, before this change only the Id and Selection properties were correct in the changed view sent to the plugin, all the other properties were either empty like OptionKeys, or filled with incorrect data like Name or Options. After this change all properties are correct. Still not sure if this change will be included in the next release, but it's a good idea to fix your plugins no matter what so that they use .Id instead of .Name

[stefxx]

Agree. I've updated all my affected plugins and they are submitted for release.

[mnsandler]

I will check in early January and post back.

[spudwebb]

@alexbk66 did you get a chance to test the 4.2.16.4 build posted above? @stefxx does your plugins with the fix for .Id vs .Name got approved and released? @mnsandler did you get a chance to test your actions and triggers? thanks

[stefxx]

@stefxx does your plugins with the fix for .Id vs .Name got approved and released?

Yes they did!

[alexbk66]

@alexbk66 did you get a chance to test the 4.2.16.4 build posted above?

I'm away for another week, then I'll test

[spudwebb]

@alexbk66 I tested AK GoogleCast plugin with the 4.2.16.6 build https://homeseer.com/updates4/SetupHS4_4_2_16_6.msi The select lists on the settings page and device config page do not work: when I select an option and save, the select list reset to the previous option selected. I would need your help to know exactly what properties/methods of SelectListView you are using in OnSettingPageSave / OnDeviceConfigChange that is causing the problem. Thanks

[alexbk66]

Ok, I'll check

[wvuyk]

After installing HS4.2.17.4 the PluginSDK 1.4.3 is missing here on Windows.

Where do I find the dll eventually?

[spudwebb]

I had the same error, running the installer again and doing a repair fix it for me.

[alexbk66]

First unpleasant surprise - I tried to install the new version side-by-side with the existing HS4 install (C:\Program Files (x86)**HomeSeer HS4\Bin\homeseer) - to keep the original version. But the new installer removed the files from the original folder first, then installed in C:\Program Files (x86)\HomeSeer 4_2_16_6**\Bin\homeseer\

Why?

[rjhelmke]

First unpleasant surprise - I tried to install the new version side-by-side with the existing HS4 install (C:\Program Files (x86)HomeSeer HS4\Bin\homeseer) - to keep the original version. But the new installer removed the files from the original folder first, then installed in C:\Program Files (x86)_HomeSeer 4_2_166\Bin\homeseer\

Why?

The windows installer will always try to update an existing install, so you cannot use the installer to install 2 copies of the application. What I do is run a Windows Sandbox, then I can install another copy of HS. You can also copy the entire HS4 folder to another location to get another install, but the installer will never touch this copy so you will need to update files manually.

[alexbk66]

I tested the 4.2.16.6 - and obviously it's broken. I made changes which should work with both HS versions:

[alexbk66]

I posted AKGoogleCast BETA 4.0.5.0 which should work with both HS versions.

I must admit, introducing breaking changes to fix something was asked more than two years ago? What a mess it creates - if developers don't update every plugin - they will be broken.

I admit the original implementation of SelectListView was pretty ugly, but you don't break existing code 2 years later. Usual practice is to add new methods to the interface, not change existing signatures.

I have to rebuild and re-publish my 16 plugins now.

[spudwebb]

@alexbk66, in your old code if you would have used directly the Selection property like this UpdateSelectOld(currentView, changedView.Selection.ToString()) Then it would have worked in both versions without you having to make any changes With the old behavior changedView.Selection.ToString() and changedView.Options[changedView.Selection] were the same thing, so not sure why you used the latter?

I agree on not introducing breaking changes, but in this specific case the implementation of SelectListView did not change at all, it's the way the changed view data is passed from HS to the plugin that has changed, so it wasn't possible to just add new methods or properties to SelectListView to fix the problem. We decided to fix this issue because it is needed for some new enhancements of SelectListView, and because this issue had confused a lot of plugin developers and was likely to confuse all new future plugin developers.

Anyway thanks for the fix in AKGoogleCast, please fix your other plugins and submit them and we will approve and release them ASAP.

[alexbk66]

@spudwebb I'll check your suggestion

[EDIT]

Probably it was trial-and- error I guess, it was long time ago

[alexbk66]

BTW, one question. I noticed in my code above I use currentView.OptionKeys, not changedView.OptionKeys - is it better to use changedView? Or they are always the same?

[spudwebb]

After the fix currentView.OptionKeys and changedView.OptionKeys should always be the same. Before the fix changedView.OptionKeys was always null

In your screenshot above, the newVer case should actually work for any version because you use currentView.OptionKeys and currentView.Options

[mnsandler]

Testing with 4.2.16.6

(1) this is my one call to .GetSelectedOptionKey in my own wrapper. i have a similar wrapper for .GetSelectedOption. these seem to be working fine.

(2) As i mentioned above, I'm not using .OptionsKeys to get the full list, just to reference the specific selection. see my code below. it seems to still be working:

(3) Is there anything else i should check?

[spudwebb]

@mnsandler , you should test that you can edit and save fields (particularly SelectListView but not only) in your settings pages, device config pages, and event actions/triggers pages. If it works as expected, your code is fine.

[mnsandler]

I am using the .name property (vs .id) within OnConfigItemUpdate in my Action Class. not using it in my Trigger class

in my testing .name and .id seem to have the same value right now (with 4.2.16.7)

is .name going to change in 4.2.17.0?

[spudwebb]

in 4.2.16.7 .name and .id shouldn't have the same value within OnConfigItemUpdate, unless you used the same value for these properties when you created the view.

[mnsandler]

just to be clear, i am only appending my keyword to the page id (in this case "-Action"). The below pic is what i see in debug mode when i first create an insteon action

i never explicitly set the .id or .name property for the selectview.

i am not asking you to make any changes, just looking for clarification

[spudwebb]

I don't understand... How do you create the SelectListView ? Using either PageFactory.WithDropDownSelectList() or the SelectListView constructor, you have to pass an id and a name.

[mnsandler]

i do make a call to PageFactory.WithDropDownSelectList() but that has different values when initially created. see the pic

i guess i dont understand where the .name property on the above selectlistview is set since i dont do it explicitly (as mentioned above)

maybe we are in a rabbit hole.

if you didn't make any changes to the .name property and my plugin is working with 4.2.16.7 then i think I'm good.

i can always change from .name to .id if that is more correct

[spudwebb]

I think you are not using the correct PluginSdk version at runtime when you debug. Are you directly launching your plugin from visual studio to debug? If so you need to make sure to update the PluginSdk.dll in your plugin solution to the same version than the one used by HS 4.2.16.7, which is version 1.4.3

The behavior you describe is exactly what we fixed in HS 4.2.16.7 / PluginSdk 1.4.3, the .name property was set to the .id value before the view change is passed to your plugin in OnConfigItemUpdate, now in HS 4.2.16.7 / PluginSdk 1.4.3 the .name property should be correct (i.e. in your case name = "Insteon Actions")

So if you used the name property in your code and expect it to be the id, you need to change that and use the id property.

[mnsandler]

i was able to update my project references to pluginsdk.dll 1.4.3

however i can't update to hscf.dll 1.0.0.3. thus i can't compile. i've tried removing the old reference, closing and reopening the project and then adding back in the new reference. i tried both files (the one in the root and the one in the bin) still not working.

any suggestions?

[spudwebb]

You don't need to update HSCF.dll, there has been no change to this dll for a long time. You see version 1.0.0.2 in Visual Studio because for some reason this dll has an assembly version (1.0.0.2) different from its assembly file version (1.0.0.3)

What compilation error do you get?

[mnsandler]

[mnsandler]

do i need to load the plugin sdk source code? i normally don't do this

[mnsandler]

nvm. i added the following and it seems to have resolved these issues.

Imports HomeSeer.PluginSdk