search

dpaezi / mytracks

Automatically exported from code.google.com/p/mytracks
1 stars 2 forks source link

Power, Bike Speed, Cadence, Heart Rate and Export to GoldenCheetah #756

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Mercurial clone with changes to be reviewed:
code.google.com/r/chet0xhenry-mytracks/

Revisions to be pulled from that clone (or "all"):
ALL

Purpose of code changes on the clone:
1. Merged all ant+ devices into the AntDirectSensorManager.java
   with the intent of removing the SRM class.

2. Added code in the above class to handle standard power messages
  A. Calculates Cadence from above message
  B. Calculates Speed from wheel rotations from above message
  C. Calculates Power from std power message
  D.  TODO: Someone with SRM should check to see that it still pairs
            and uses the SRM message type in the new merged class
  E.  TODO: If SRM works remove the old SRM class.

3. Added higher precision storage of the ant calculated values
  A.  TODO:  These values should really be stored in a supporting
             table.  This table would have a foreign key to the current
             table and would only insert a new row if we have sensor
             data.  This would also allow for easyer additions to the
             said table as needed and lower storage requirements not
             to mention better reporting.
4. Added Ant+ pairing settings for each sensor type
  A.  TODO: This really needs reworked
    I. Use a tree starting with sensor type (Ant, Blue...) then show
       the sensors and allow the user to either enter the id or search
       for each sensor type.

5. Added export to GC file type (http://goldencheetah.org/)

When reviewing my code changes, please focus on:
Power, Bike Speed, Cadence, Heart Rate, Export to GC

Did you run the unit tests?
No, I having problems with building mytrackstest:
  Internal compiler error: java.lang.NoClassDefFoundError:
    TripStatistics at 
  java.lang.Class.getDeclaredMethods0(Native Method)

Did you allow others to review code on your clone?
Yes.

Original issue reported on code.google.com by chet.0x....@gmail.com on 17 Jan 2012 at 5:00

GoogleCodeExporter commented 9 years ago 
I looked at your branch and I saw a huge number of commits.  I would ask you to 
either collapse all of the changes with a tool or by hand.
I might actually suggest that you split this up into many smaller commits.  It 
is much easier for people to review smaller logical chunks.

Original comment by sandordo...@google.com on 17 Jan 2012 at 5:07

GoogleCodeExporter commented 9 years ago 
I'll work on it after work tonight.  I think that there would be two to four 
logical groupings of commits.  Most of the changes are in one file so the 
review should not be to bad after I group up the commits.  Will that be ok?

Original comment by chet.0x....@gmail.com on 17 Jan 2012 at 5:41

GoogleCodeExporter commented 9 years ago 
That sounds fine.  I might suggest submitting one or at most two commits at a 
time.   Please keep each logical change in it's own clone. If one change 
depends on another don't send the former until the later has been approved.

Original comment by sandordo...@google.com on 17 Jan 2012 at 5:55

GoogleCodeExporter commented 9 years ago 
Ok, I have grouped the code into three commits in a new clone:
code.google.com/r/chet0xhenry-antplus/

The first commit has almost all of the changes in the mytracks folder except 
for the goldencheetah export class, however there are calls to that class so 
the second commit is also required.  I would have liked to split it up with the 
ant+ code in one commit and the goldencheetah code in another.  This however 
proved to be too time consuming because the changes were altering the same 
files.  Most of the code however is ant+ and that should be the focus.

The last commit is the code need for the mytracks-lib.  This should all be code 
related to ant+.

Original comment by chet.0x....@gmail.com on 18 Jan 2012 at 6:07

GoogleCodeExporter commented 9 years ago 
These commits are still huge and hard to review. Putting it all in one clone 
makes it very hard to have address comments as I have to look several commits 
later for your response.

Original comment by sandordo...@google.com on 20 Jan 2012 at 5:21

GoogleCodeExporter commented 9 years ago 
You aslo did not allow comments on your clone. Please do this again but only 
put one small initial change in that clone.  We can work from there to the 
final change.  Also avoid pointless format changes like:
http://code.google.com/r/chet0xhenry-antplus/source/diff?format=side&path=/MyTra
cks/res/layout/sensor_state.xml&r=4e0a17ac73dd4b770cfaa6243b48413067b49f18&spec=
svn4e0a17ac73dd4b770cfaa6243b48413067b49f18

Original comment by sandordo...@google.com on 20 Jan 2012 at 5:25

GoogleCodeExporter commented 9 years ago 
I realize that this is a large change however the changes are for one feature, 
the ability to read ant+ standard power messages and use the information to 
calculate power, cadence, and speed (bSpeed).  

Unfortunately, this makes almost everything dependent.  The above format 
changes was not intended to be useless.  The bSpeed is the measured speed from 
ant+ standard power message it uses the event period and the wheel rpm to give 
us an accurate measurement of speed and distance.  This speed is also more 
reliable than gps.  Are you saying that if we have an ant+ power sensor we 
should replace gps speed with the ant measurement?  I personally would like to 
record both and maybe only display one.  Maybe we could use the bSpeed 
measurement to improve the accuracy of the gps, along with acceleration, and 
magnetic field?

The one thing that could be broken out is the golden cheetah export, but that 
may take a few hours.

There is one more todo I would like to take on this weekend, currently the 
bSpeed is not localized and only reports in miles per hour.

I update the clone to allow non-members to review code, I'm sorry I missed that 
step.

Original comment by chet.0x....@gmail.com on 20 Jan 2012 at 5:52

GoogleCodeExporter commented 9 years ago 
Not everything in your change is dependent.  There are fairly clear lines to 
separate the changes. I do like the changes that you are proposing but we have 
to do this in a controlled manner.  Please create a clone with 1 commit and 
only one commit.  In that commit put one logical change. Perferably less that 5 
changed or added classes.
I would suggest that you do the GoldenCheetah export first as another 
contributor is radically reworking the ANT+ sensor work.  I like how he has 
changed the classes and will likely take his contribution.  He did not however 
implement power or speed sensors which I would like to take from you. However 
since his change is not complete I would encourage you to work on the export 
code as that should be compartmentalized.  Then once the new ANT+ changes are 
in you can integrate with that framework to add power and speed. 
You might want to look at:
http://code.google.com/r/ezerotven-mytracks2/source/list
It shows where I would like to take the ant sensor code.  I also shows good 
development style as each commit is small and atomic.

Original comment by sandordo...@google.com on 24 Jan 2012 at 6:02

GoogleCodeExporter commented 9 years ago 
Jimmy these changes LGTM.  I think it is ok to pull it into head and maybe even 
the latest release.

Original comment by sandordo...@google.com on 31 Jan 2012 at 6:14

GoogleCodeExporter commented 9 years ago 
Jimmy these changes LGTM.  I think it is ok to pull it into head and maybe even 
the latest release.

Original comment by sandordo...@google.com on 31 Jan 2012 at 6:14

GoogleCodeExporter commented 9 years ago 
Oops. Jimmy ignore my previous message. Chet the other ant changes will be 
committed soon. Please look at how you can add power in that framework.

Original comment by sandordo...@google.com on 31 Jan 2012 at 6:27

GoogleCodeExporter commented 9 years ago 
Direct connection to ANT+ power sensors and speed from odometry are important 
additions that I'm keen to use. The recent additions that allow direct 
connection to ANT cadence sensors are not enough. I'm wondering how this is 
going.

Original comment by kenep...@gmail.com on 13 Mar 2012 at 1:58

GoogleCodeExporter commented 9 years ago 
Also can you please include the distance as calculated by the speed sensor in 
the exported TCX files as described at 
http://developer.garmin.com/schemas/tcx/v2/xmlspy/index.html#element_Trackpoint_
Link05E0A1B0 and in the exported CSV files.

Original comment by kenep...@gmail.com on 13 Mar 2012 at 3:16

GoogleCodeExporter commented 9 years ago 
Currently, we support Ant+ heart rate monitors and Ant+ speed distance 
monitors. Please file new feature request for add other Ant+ profiles. Going to 
close this bug.

Original comment by jshih@google.com on 5 Jun 2012 at 12:01

GoogleCodeExporter commented 9 years ago 
A new code review request with this feature was created:
http://code.google.com/p/mytracks/issues/detail?id=1023

Original comment by chet.0x....@gmail.com on 9 Jul 2012 at 2:44