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

FeatureFactory.AddGraphic incorrect check for range vs single value #291

Closed pseudocoder closed 2 years ago

pseudocoder commented 2 years ago

The second IF statement in this block should also check that the statusGraphic is NOT a range, otherwise the single value in a range is checked when it should be ignored.

if (statusGraphic.IsRange && _feature.HasGraphicForRange(statusGraphic.TargetRange))
{
    throw new ArgumentException("A value targeted by the specified statusGraphic already has a graphic bound to it.", "statusGraphic");
}

if (_feature.HasGraphicForValue(statusGraphic.Value))
{
    throw new ArgumentException("The value targeted by the specified statusGraphic already has a graphic bound to it.", "statusGraphic");
}

suggest replacing with:

if (statusGraphic.IsRange)
{
    if (_feature.HasGraphicForRange(statusGraphic.TargetRange))
    {
        throw new ArgumentException("A value targeted by the specified statusGraphic already has a graphic bound to it.", "statusGraphic");
    }
}
else
{
    if (_feature.HasGraphicForValue(statusGraphic.Value))
    {
        throw new ArgumentException("The value targeted by the specified statusGraphic already has a graphic bound to it.", "statusGraphic");
    }
}

this also gives clear space for any future range / non-range checking.

pseudocoder commented 2 years ago

ps this may only occur when there is a single value (not a range) for ZERO; workaround is to add that after any ranges.

spudwebb commented 2 years ago

this problem has been fixed in version 1.4.1, check the current code: https://github.com/HomeSeer/Plugin-SDK/blob/3bacebb4a7bf3fd27e0fd04eb252c323b7d55f06/Devices/FeatureFactory.cs#L703-L708

pseudocoder commented 2 years ago

Thanks - I should have checked the version bundled in the sample plugin, which is 1.3.xxx