Adressing a round of review comments. Tests in separate commit.

MyTracks/src/com/google/android/apps/mytracks/services/sensors/StrideReadings.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2010 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.android.apps.mytracks.services.sensors;
+
+import java.util.LinkedList;
+import java.util.List;
+
+/**
+ * Storage of a history of readings of the Zephyr stride counter,
+ * in order to derive a correct cadence value from it,
+ * working around an issue with the HxM's firmware. 
+ *
+ * @author Dominik Ršttsches
+ */
+public class StrideReadings {
+  private static class StrideReading {
+    public int timeMs;
+    public int numStrides;
+
+    StrideReading(int newTimeMs, int newNumStrides) {
+      timeMs = newTimeMs;
+      numStrides = newNumStrides;
+    }
+  }
+
+  private static final int NUM_READINGS_FOR_AVERAGE = 10;
+  private static final int MIN_READINGS_FOR_AVERAGE = 5;
+
+  protected static final int CADENCE_NOT_AVAILABLE = -1;
+
+  private List<StrideReading> strideReadingsHistory;
+
+  public StrideReadings() {
+    strideReadingsHistory = new LinkedList<StrideReading>();
+  }
+
+  public void updateStrideReading(int timeInMs, int numStrides) {
+    // HRM/HxM documentation says, transmission frequency is 1 Hz, 
+    // let's keep last NUM_READINGS_FOR_AVERAGE readings.
+    // TODO: Calibrate this using a reliable footpod / cadence sensor, 
+    // otherwise use heartbeat timestamp for calculation. 
+    strideReadingsHistory.add(0, new StrideReading(timeInMs, numStrides));
+    while(strideReadingsHistory.size() > NUM_READINGS_FOR_AVERAGE) {
+      strideReadingsHistory.remove(strideReadingsHistory.size()-1);
+    }
+  }
+
+  public int getCadence() {
+    if(strideReadingsHistory.size() < MIN_READINGS_FOR_AVERAGE) {
+      // Bail out if we cannot really get a meaningful average yet.
+      return CADENCE_NOT_AVAILABLE;
+    }
+    // compute assuming 1 stride reading/second
+    int timeSinceOldestReadingSecs = strideReadingsHistory.size() - 1; 
+    int stridesThen = strideReadingsHistory
+      .get(strideReadingsHistory.size()-1).numStrides;
+    int stridesNow = strideReadingsHistory
+      .get(0).numStrides;
+    // Contrary to documentation stride value seems to roll over every 127 strides.
+    return Math.round( (float)((stridesNow - stridesThen) % 127) / 
+        timeSinceOldestReadingSecs * 60);
+  }
+}

MyTracks/src/com/google/android/apps/mytracks/services/sensors/ZephyrMessageParser.java

@@ -1,6 +1,5 @@
 /*
  * Copyright 2010 Google Inc.
- * Copyright 2011 Dominik Ršttsches <d.roettsches@gmail.com>
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
@@ -21,12 +20,11 @@ import com.google.android.apps.mytracks.content.Sensor;
 
 import android.util.Log;
 
-import java.util.LinkedList;
-
 /**
  * An implementation of a Sensor MessageParser for Zephyr.
  *
  * @author Sandor Dornbush
+ * @author Dominik Ršttsches
  */
 public class ZephyrMessageParser implements MessageParser {
 
@@ -34,54 +32,10 @@ public class ZephyrMessageParser implements MessageParser {
   public static final int ZEPHYR_HXM_BYTE_CRC = 58;
   public static final int ZEPHYR_HXM_BYTE_ETX = 59;
   
+  private static final String CADENCE_BUG_FW_ID = "1A00316550003162";
+  
   private StrideReadings strideReadings;
   
-  public class StrideReadings {
-    class StrideReading {
-      public int timeMs;
-      public int numStrides;
-      
-      StrideReading(int newTimeMs, int newNumStrides) {
-        timeMs = newTimeMs;
-        numStrides = newNumStrides;
-      }
-    }
-    
-    private LinkedList<StrideReading> strideReadingsHistory;
-    private static final int NUM_READINGS_FOR_AVERAGE = 10;
-    private static final int MIN_READINGS_FOR_AVERAGE = 5;
-    public static final int CADENCE_NOT_AVAILABLE = -1;
-    
-    public StrideReadings() {
-      strideReadingsHistory = new LinkedList<StrideReading>();
-    }
-
-    public void updateStrideReading(int timeInMs, int numStrides) {
-      // HRM/HxM docs say, transmission frequency is 1 Hz, 
-      // let's keep last NUM_READINGS_FOR_AVERAGE readings.
-      // TODO: Calibrate this using a reliable footpod / cadence sensor, 
-      // otherwise perhaps use heartbeat timestamp for calculation. 
-      strideReadingsHistory.addFirst(new StrideReading(timeInMs, numStrides));
-      while(strideReadingsHistory.size() > NUM_READINGS_FOR_AVERAGE) {
-        strideReadingsHistory.removeLast();
-      }
-    }
-    
-    public int getCadence() {
-      if(strideReadingsHistory.size() < MIN_READINGS_FOR_AVERAGE) {
-        // Bail out if we cannot really get a meaningful average yet.
-        return CADENCE_NOT_AVAILABLE;
-      }
-      // compute assuming 1 stride reading/second
-      int timeSinceOldestReadingSecs = strideReadingsHistory.size() - 1; 
-      int stridesThen = strideReadingsHistory.getLast().numStrides;
-      int stridesNow = strideReadingsHistory.getFirst().numStrides;
-      // Contrary to documentation stride value seems to roll over every 127 strides.
-      return Math.round( (float)((stridesNow - stridesThen) % 127) / 
-          timeSinceOldestReadingSecs * 60);
-    }
-  }
-  
   @Override
   public Sensor.SensorDataSet parseBuffer(byte[] buffer) {
     StringBuilder sb = new StringBuilder();
@@ -93,29 +47,45 @@ public class ZephyrMessageParser implements MessageParser {
     // Device Firmware ID, Firmware Version, Hardware ID, Hardware Version
     // 0x1A00316550003162 produces erroneous values for Cadence and needs
     // a workaround based on the stride counter.
+    // Firmware values range from field 3 to 11 of the byte buffer, 
+    // since in hex there are two characters for each byte, doubling the values.
     String hardwareFirmwareId = sb.substring(6, 22);
-    boolean needsWorkaround = hardwareFirmwareId.equals("1A00316550003162");
+    boolean computeCadenceFromStrides = hardwareFirmwareId.equals(CADENCE_BUG_FW_ID);
-    Log.w(Constants.TAG, "FW & HW Ids & Version " + hardwareFirmwareId + " needs workaround: " + needsWorkaround);
+    Log.d(Constants.TAG, "FW & HW Ids & Version " + hardwareFirmwareId + " needs workaround: " + computeCadenceFromStrides);
+    
+    Sensor.SensorDataSet.Builder sds =
+      Sensor.SensorDataSet.newBuilder()
+      .setCreationTime(System.currentTimeMillis());
 
     Sensor.SensorData.Builder heartrate = Sensor.SensorData.newBuilder()
       .setValue(buffer[12] & 0xFF)
       .setState(Sensor.SensorState.SENDING);
+    sds = sds.setHeartRate(heartrate);
     
     Sensor.SensorData.Builder batteryLevel = Sensor.SensorData.newBuilder()
       .setValue(buffer[11])
       .setState(Sensor.SensorState.SENDING);
+    sds = sds.setBatteryLevel(batteryLevel);
+    
+    // Appends cadence to SensorDataSet builder if available.
+    parseOrComputeCadence(sds, buffer, computeCadenceFromStrides);
+    
+    return sds.build();
+  }
+
+  private void parseOrComputeCadence(Sensor.SensorDataSet.Builder sds, byte[] buffer, boolean computeFromStrides) {
     
     Sensor.SensorData.Builder cadence = Sensor.SensorData.newBuilder();
 
-    if(!needsWorkaround) {
+    if(!computeFromStrides) {
       cadence = cadence
         .setValue(SensorUtils.unsignedShortToIntLittleEndian(buffer, 56) / 16)
         .setState(Sensor.SensorState.SENDING);
+      sds.setCadence(cadence);
     } else {
       if(strideReadings == null) {
         strideReadings = new StrideReadings();
       }
-  
       strideReadings.updateStrideReading(
           SensorUtils.unsignedShortToIntLittleEndian(buffer, 14), 
           buffer[54] & 0xFF);
@@ -123,20 +93,9 @@ public class ZephyrMessageParser implements MessageParser {
       if(strideReadings.getCadence() != StrideReadings.CADENCE_NOT_AVAILABLE) {
         cadence = cadence.setValue(strideReadings.getCadence())
           .setState(Sensor.SensorState.SENDING);
-      } else {
+        sds.setCadence(cadence);
-        cadence = cadence.setValue(0).setState(Sensor.SensorState.NONE);
       }
     }
-      
-    Sensor.SensorDataSet sds =
-      Sensor.SensorDataSet.newBuilder()
-      .setCreationTime(System.currentTimeMillis())
-      .setBatteryLevel(batteryLevel)
-      .setHeartRate(heartrate)
-      .setCadence(cadence)
-      .build();
-    
-    return sds;
   }
 
   @Override