Missing "custom_target_value_low" field for "heart_rate" target_type
First of all, thanks for a great lib. Most things are working great but found an issue.
The issue
I'm importing a structured workout .fit file where a workout step has target_type: "heart_rate" and a "custom_target_value_low"=240 & custom_target_value_high="260".
When viewing the file through https://www.fitfileviewer.com/ it shows these fields.

In the README.md of this project it recommends FIT File Explorer, and when using that tool it also shows these fields, but named custom_target_heart_rate_high, which actually might be more correct when looking at documentation (https://developer.garmin.com/fit/file-types/workout/#targettype).

But when using this lib, FitFileParser, they are not included. Instead there's a target_hr_zone with a value of 0.

Steps to reproduce
- Create a new swift project
- Write the following code:
if let fit = FitFile(file: selectedFile) {
let steps = fit.messages(forMessageType: .workout_step)
for step in steps {
print(step.interpretedFieldKeys())
}
}
- Use the attached .fit file and run
- Notice the output is:
["target_hr_zone", "duration_time", "intensity", "message_index_value", "target_type", "duration_type", "wkt_step_name", "message_index"]
["message_index_value", "duration_type", "intensity", "message_index"]
Expected to happen
The output should include the custom_target_value_low/custom_target_heart_rate_low and custom_target_value_high/custom_target_heart_rate_high fields.
.fit file I use
An interesting finding. It looks like the issue happened between version 1.4.5 and 1.4.6 because if I change the Swift package to use version 1.4.5 and run the following code (same as before):
if let fit = FitFile(file: selectedFile) {
let steps = fit.messages(forMessageType: .workout_step)
for step in steps {
print(step.interpretedFieldKeys())
}
}
I now instead get the following (expected) output (notice how custom_target_heart_rate_high + custom_target_heart_rate_low is included now):
["target_hr_zone", "custom_target_heart_rate_high", "message_index", "duration_type", "duration_time", "wkt_step_name", "custom_target_heart_rate_low", "intensity", "target_type"]
["intensity", "duration_type", "message_index"]
As soon as I go up to 1.4.6 or higher this starts failing as described initially.
Next finding! If I change the code to the following (parsingType: .generic, with the latest version, 1.4.8) it sort of works:
if let fit = FitFile(file: selectedFile, parsingType: .generic) {
let steps = fit.messages(forMessageType: .workout_step)
for step in steps {
print(step.interpretedFieldKeys())
print(step.interpretedFields())
}
}
The interpreted field looks like this:
"custom_target_heart_rate_high": FitField(withName: workout_hr_260)
Compared to the one in version 1.4.5:
"custom_target_heart_rate_high": FitField(withValue: 260.0, andUnit: % or bpm)
So I can get the value now in 1.4.8 but I get it as a String (workout_hr_260) that I have to parse, instead of as a Double (260.0) as I can get in 1.4.5.
Thanks for the detailed issue. I understand this issue, it has to do with the interpretation of the profile.xls, in the case of this field, it's a special type that says:
0 - 100 indicates% of max hr; >100 indicates bpm (255 max) plus 100
I will add logic for that. This was introduced in the new version as it included a fix to handle definition with masked value like left_balance. workout_hr appears initially like a masked value but it's not so it confuses the logic...
I'll add a fix
do you mind if I use your example file and include it as a test case?
Go ahead, it's a simple test file for me too.
pushed a fix, thanks for the bug. let me know if you still have some issues
Note also that it does not really get the unit resolved, the specification for field with offset in the profile file is a bit incomplete, and I already had to do some specific handling for the workout_hr type, so I felt it was not worth make the code overly specialised to handle a unit that would be dependent on another field like in that case and kept it as "% or bpm"
Awesome!
I noticed one thing though, I now get the value with offset already applied. So in this case I get "custom_target_heart_rate_low" as 140 but it should be 240 so that I can use my own logic to determine whether it's relative or absolute, as indicated on https://developer.garmin.com/fit/file-types/workout/#settingpowerandheartratevalues.
Same for power, custom_target_power_lowwas in the file set as 1100 (to indicate an absolute 100 value) but I now get it as 100 triggering my code to believe it's a relative value instead.
I think "rzfit_swift_value_from_workout_power" and "rzfit_swift_value_from_workout_hr" should probably just return the value as is to be more in line with the documentation.
Hum. you make a very good point. but your message got me a bit confused as to how I should deal with it :)
The library attempts to interpret as much as possible based on the Profile.xlsx, so for example it will convert the value to the fields based on reference fields, apply the masks, etc.
For the mask, the library separate the interpretation of the mask and the value, rather than leaving the raw value containing the mask as well. (for example left_right_balance)
I felt for consistency I should do the same for offset rather than just return the raw value. But because the offset fields seem more like a value, and the interpretation a unit, instead of using _value, I use _unit to store what unit it was in.
So you can find the field custom_target_heart_rate_high and custom_target_heart_rate_high_unit which will contain % or bpm.
Maybe not be exactly what you were after, but I think it solves your problem and it keeps the library behaviour consistent. Fair?
Sounds like it should work. For HR any value above 100 is considered absolute (offset by 100) and below relative. So a value of 70 would be 70% of max HR and 170 would be exactly 70 BPM, if I understand correctly.
I don't know the ins and outs of this lib's code but it looked like rzfit_swift_value_from_workout_hr would give a value of -30 (70-100, i.e. minus the absolute offset) instead of 70 if the raw value was 70.
Please do correct me if I'm wrong though.
You were right, but no longer :) I fixed it in the last commit...
So for HR now:
- if custom_target_heart_rate_high = 70 you will get custom_target_heart_rate_high = 70 and custom_target_heart_rate_high_unit = %
- if custom_target_heart_rate_high = 250 you will get custom_target_heart_rate_high = 150 and custom_target_heart_rate_high_unit = bpm
Hope I got it right this time. I don't know if you know how to generate a file with % as an input so I could test it?
Anyway, I'll get there eventually... thanks for your patience...
I see, terrific!
My test file was generated from a service that only deals with absolute values. Could try looking around for one dealing with relative ones.
Don't worry then, it was just if easy to generate.
Let me know if latest version works for you and I will close the issue
Almost!
return input < offset ? Double(input) : Double(input - offset)
should be
return input <= offset ? Double(input) : Double(input - offset)
I.e. 100 = 100% and 101 = 1bpm for HR, and 1000 = 100% and 1001 = 1power for power.