Repository: bpicode/fritzctl (go)

Issue:
fritzctl list switches
fritzctl list switches is also listing thermostats. Im pretty sure that shouldn´t be.

Pull request:
Issue #59: use function bitmask to determine device type.
Closes #59 

Patch:
diff --git a/fritz/device.go b/fritz/device.go
index 2ff5334..7765c79 100644
--- a/fritz/device.go
+++ b/fritz/device.go
@@ -1,6 +1,8 @@
 package fritz
 
-import "strings"
+import (
+	"strconv"
+)
 
 // Device models a smart home device. This corresponds to
 // the single entries of the xml that the FRITZ!Box returns.
@@ -24,10 +26,18 @@ type Device struct {
 
 // IsSwitch returns true if the device is recognized to be a switch and returns false otherwise.
 func (d *Device) IsSwitch() bool {
-	return d.Powermeter.Power != "" || d.Powermeter.Energy != "" || strings.Contains(d.Productname, "FRITZ!DECT")
+	return d.hasMask(512)
 }
 
 // IsThermostat returns true if the device is recognized to be a HKR device and returns false otherwise.
 func (d *Device) IsThermostat() bool {
-	return d.Thermostat.Measured != "" || d.Thermostat.Goal != "" || d.Thermostat.Saving != "" || d.Thermostat.Comfort != "" || strings.Contains(d.Productname, "Comet DECT")
+	return d.hasMask(64)
+}
+
+func (d *Device) hasMask(mask int64) bool {
+	bitMask, err := strconv.ParseInt(d.Functionbitmask, 10, 64)
+	if err != nil {
+		return false
+	}
+	return (bitMask & mask) != 0
 }


Test patch:
diff --git a/fritz/device_test.go b/fritz/device_test.go
new file mode 100644
index 0000000..9778004
--- /dev/null
+++ b/fritz/device_test.go
@@ -0,0 +1,18 @@
+package fritz
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestParsingFunctionBitMask verifies correctness of the function bit.mask interpretation.
+func TestParsingFunctionBitMask(t *testing.T) {
+	assertions := assert.New(t)
+	assertions.False((&Device{Functionbitmask: "nonsense"}).IsSwitch())
+	assertions.False((&Device{Functionbitmask: "nonsense"}).IsThermostat())
+	assertions.True((&Device{Functionbitmask: "2944"}).IsSwitch())
+	assertions.False((&Device{Functionbitmask: "2944"}).IsThermostat())
+	assertions.False((&Device{Functionbitmask: "320"}).IsSwitch())
+	assertions.True((&Device{Functionbitmask: "320"}).IsThermostat())
+}
diff --git a/fritz/devicelist_test.go b/fritz/devicelist_test.go
index fec09bb..c4ffe9a 100644
--- a/fritz/devicelist_test.go
+++ b/fritz/devicelist_test.go
@@ -23,3 +23,19 @@ func TestSwitchAndThermostatFiltering(t *testing.T) {
 
 	assert.Equal(t, len(l.Devices), len(l.Switches())+len(l.Thermostats()))
 }
+
+// TestSwitchAndThermostatFilteringIssue56 reproduces https://github.com/bpicode/fritzctl/issues/59.
+func TestSwitchAndThermostatFilteringIssue56(t *testing.T) {
+	f, err := os.Open("../testdata/devicelist_issue_59.xml")
+	assert.NoError(t, err)
+	defer f.Close()
+
+	var l Devicelist
+	err = xml.NewDecoder(f).Decode(&l)
+	assert.NoError(t, err)
+
+	assert.Len(t, l.Thermostats(), 4)
+	assert.Len(t, l.Switches(), 8)
+
+	assert.Equal(t, len(l.Devices), len(l.Switches())+len(l.Thermostats()))
+}
diff --git a/testdata/devicelist_issue_59.xml b/testdata/devicelist_issue_59.xml
new file mode 100644
index 0000000..51defc3
--- /dev/null
+++ b/testdata/devicelist_issue_59.xml
@@ -0,0 +1,268 @@
+<devicelist version="1">
+    <device identifier="1" id="25" functionbitmask="2944" fwversion="03.87" manufacturer="AVM"
+            productname="FRITZ!DECT 200">
+        <present>1</present>
+        <name>S1</name>
+        <switch>
+            <state>1</state>
+            <mode>manuell</mode>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+        </switch>
+        <powermeter>
+            <power>8510</power>
+            <energy>15324</energy>
+        </powermeter>
+        <temperature>
+            <celsius>250</celsius>
+            <offset>0</offset>
+        </temperature>
+    </device>
+    <device identifier="2" id="26" functionbitmask="2944" fwversion="03.87" manufacturer="AVM"
+            productname="FRITZ!DECT 200">
+        <present>1</present>
+        <name>S2</name>
+        <switch>
+            <state>0</state>
+            <mode>auto</mode>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+        </switch>
+        <powermeter>
+            <power>0</power>
+            <energy>5383</energy>
+        </powermeter>
+        <temperature>
+            <celsius>235</celsius>
+            <offset>0</offset>
+        </temperature>
+    </device>
+    <device identifier="3" id="22" functionbitmask="2944" fwversion="03.87" manufacturer="AVM"
+            productname="FRITZ!DECT 200">
+        <present>1</present>
+        <name>S3</name>
+        <switch>
+            <state>1</state>
+            <mode>auto</mode>
+            <lock>0</lock>
+            <devicelock>1</devicelock>
+        </switch>
+        <powermeter>
+            <power>90980</power>
+            <energy>276747</energy>
+        </powermeter>
+        <temperature>
+            <celsius>270</celsius>
+            <offset>0</offset>
+        </temperature>
+    </device>
+    <device identifier="4" id="18" functionbitmask="2944" fwversion="03.87" manufacturer="AVM"
+            productname="FRITZ!DECT 200">
+        <present>1</present>
+        <name>S4</name>
+        <switch>
+            <state>0</state>
+            <mode>auto</mode>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+        </switch>
+        <powermeter>
+            <power>0</power>
+            <energy>5732</energy>
+        </powermeter>
+        <temperature>
+            <celsius>220</celsius>
+            <offset>0</offset>
+        </temperature>
+    </device>
+    <device identifier="5" id="21" functionbitmask="2944" fwversion="03.87" manufacturer="AVM"
+            productname="FRITZ!DECT 200">
+        <present>1</present>
+        <name>S5</name>
+        <switch>
+            <state>1</state>
+            <mode>auto</mode>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+        </switch>
+        <powermeter>
+            <power>30460</power>
+            <energy>24795</energy>
+        </powermeter>
+        <temperature>
+            <celsius>255</celsius>
+            <offset>0</offset>
+        </temperature>
+    </device>
+    <device identifier="6" id="24" functionbitmask="2944" fwversion="03.87" manufacturer="AVM"
+            productname="FRITZ!DECT 200">
+        <present>1</present>
+        <name>S6</name>
+        <switch>
+            <state>0</state>
+            <mode>auto</mode>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+        </switch>
+        <powermeter>
+            <power>0</power>
+            <energy>18779</energy>
+        </powermeter>
+        <temperature>
+            <celsius>245</celsius>
+            <offset>0</offset>
+        </temperature>
+    </device>
+    <device identifier="7" id="23" functionbitmask="2944" fwversion="03.87" manufacturer="AVM"
+            productname="FRITZ!DECT 200">
+        <present>1</present>
+        <name>S7</name>
+        <switch>
+            <state>1</state>
+            <mode>manuell</mode>
+            <lock>0</lock>
+            <devicelock>1</devicelock>
+        </switch>
+        <powermeter>
+            <power>2500</power>
+            <energy>113093</energy>
+        </powermeter>
+        <temperature>
+            <celsius>235</celsius>
+            <offset>0</offset>
+        </temperature>
+    </device>
+    <device identifier="8" id="17" functionbitmask="320" fwversion="04.35" manufacturer="AVM"
+            productname="FRITZ!DECT 300">
+        <present>1</present>
+        <name>H1</name>
+        <temperature>
+            <celsius>205</celsius>
+            <offset>-30</offset>
+        </temperature>
+        <hkr>
+            <tist>41</tist>
+            <tsoll>36</tsoll>
+            <absenk>36</absenk>
+            <komfort>36</komfort>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+            <errorcode>0</errorcode>
+            <batterylow>0</batterylow>
+            <nextchange>
+                <endperiod>1503608400</endperiod>
+                <tchange>36</tchange>
+            </nextchange>
+        </hkr>
+    </device>
+    <group identifier="AA:BB:CC-900" id="900" functionbitmask="4160" fwversion="1.0" manufacturer="AVM" productname="">
+        <present>1</present>
+        <name>T_GROUP</name>
+        <hkr>
+            <tist>43</tist>
+            <tsoll>36</tsoll>
+            <absenk>36</absenk>
+            <komfort>36</komfort>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+            <errorcode>0</errorcode>
+            <batterylow>0</batterylow>
+            <nextchange>
+                <endperiod>1503608400</endperiod>
+                <tchange>36</tchange>
+            </nextchange>
+        </hkr>
+        <groupinfo>
+            <masterdeviceid>0</masterdeviceid>
+            <members>17,16,20,28</members>
+        </groupinfo>
+    </group>
+    <device identifier="9" id="16" functionbitmask="320" fwversion="04.35" manufacturer="AVM"
+            productname="FRITZ!DECT 300">
+        <present>1</present>
+        <name>H2</name>
+        <temperature>
+            <celsius>225</celsius>
+            <offset>0</offset>
+        </temperature>
+        <hkr>
+            <tist>45</tist>
+            <tsoll>36</tsoll>
+            <absenk>36</absenk>
+            <komfort>36</komfort>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+            <errorcode>0</errorcode>
+            <batterylow>0</batterylow>
+            <nextchange>
+                <endperiod>1503608400</endperiod>
+                <tchange>36</tchange>
+            </nextchange>
+        </hkr>
+    </device>
+    <device identifier="10" id="20" functionbitmask="320" fwversion="04.35" manufacturer="AVM"
+            productname="FRITZ!DECT 300">
+        <present>1</present>
+        <name>H3</name>
+        <temperature>
+            <celsius>220</celsius>
+            <offset>0</offset>
+        </temperature>
+        <hkr>
+            <tist>44</tist>
+            <tsoll>36</tsoll>
+            <absenk>36</absenk>
+            <komfort>36</komfort>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+            <errorcode>0</errorcode>
+            <batterylow>0</batterylow>
+            <nextchange>
+                <endperiod>1503608400</endperiod>
+                <tchange>36</tchange>
+            </nextchange>
+        </hkr>
+    </device>
+    <device identifier="11" id="28" functionbitmask="320" fwversion="04.35" manufacturer="AVM"
+            productname="FRITZ!DECT 300">
+        <present>1</present>
+        <name>H4</name>
+        <temperature>
+            <celsius>220</celsius>
+            <offset>0</offset>
+        </temperature>
+        <hkr>
+            <tist>44</tist>
+            <tsoll>36</tsoll>
+            <absenk>36</absenk>
+            <komfort>36</komfort>
+            <lock>0</lock>
+            <devicelock>0</devicelock>
+            <errorcode>0</errorcode>
+            <batterylow>0</batterylow>
+            <nextchange>
+                <endperiod>1503608400</endperiod>
+                <tchange>36</tchange>
+            </nextchange>
+        </hkr>
+    </device>
+    <device identifier="12" id="29" functionbitmask="2944" fwversion="03.87" manufacturer="AVM"
+            productname="FRITZ!DECT 200">
+        <present>1</present>
+        <name>S8</name>
+        <switch>
+            <state>1</state>
+            <mode>auto</mode>
+            <lock>0</lock>
+            <devicelock>1</devicelock>
+        </switch>
+        <powermeter>
+            <power>1710</power>
+            <energy>161</energy>
+        </powermeter>
+        <temperature>
+            <celsius>275</celsius>
+            <offset>0</offset>
+        </temperature>
+    </device>
+</devicelist>
\ No newline at end of file