• Replies 31 replies
  • Subscribers 19 subscribers
  • Views 1813 views
  • Users 0 members are here
Options
Related

UserProfile.getHeartRateZones crash when accessing return array

Michael
7 months ago

As you can see I get a crash when accessing the heart rate zones - this happens with a test app on my own device and it happens very rarely - my WF ran for a day while displaying the heartrate as complication...

Any ideas why this can happen?

1) Is it possible that UserProfile.getHeartRateZones may return null at some point?

2) can I rely on the docs regarding function return types like e.g. the on here: https://developer.garmin.com/connect-iq/api-docs/Toybox/UserProfile.html#getHeartRateZones-instance_function

It says the function can't return null - is that a guarantee?

  • 6 months ago in reply to Michael
    Michael said:
    The problem is, the class is too generic. It can handle data that comes from a complication as well as data that comes from a calculation formula... The type is too flexible. Generics would solve the issue, but otherwise I need a cast.

    What about something like this:

    class FlexValue {
  private var value as Number or ...;
  
  function valueAsNumber() as Number? {
    if (value instanceof Lang.Number) {
        return value;
    } else {
        return null; // or throw exception if you prefer (then you can remove "?" from return type)
  }
}

    You can use instanceof to check the runtime type of an object, and Monkey Types will narrow the compile-time type accordingly, as well. IOW, in this case, a cast should not be necessary for the line with "return value".

    Using instanceof to narrow an object's type is kind of a safe alternative to type casting imo.

    In the case of the call to calcHeartRateZone(), you know that you need a number, so I'm guessing this approach would work in at least that case (although it would be annoying to have to add a null check or catch an exception, either in the calling code or calcHeartRateZone()) Given that you're handling generic data though, maybe implementing null checks everywhere the data is used would be appropriate.

  • 6 months ago in reply to Michael
    Michael said:
    Found the culprit

    Thank you for following up on this!

    Now I don't have to go back and test/change all the code where I use getHeartRateZones without any kind of null checks Sweat smile

  • 6 months ago in reply to FlowState

    The problem is not that I need to cast because I don't know the type, its because I use a base class and can't define it as generic... I always know that the result will be T or ErrorData - but T is generic and is handled as Object in the base class... You can't solve that with instanceof or similar stuff, T may be a string, a number, an array... It's ok now - I just had a bug that in one cast the ErrorData instance did go through to my calc function, which it shouldn't have done...

    FlowState said:
    Now I don't have to go back and test/change all the code where I use getHeartRateZones without any kind of null checks
    • Is it now save to remove the null check for the return value of UserProfile.getHeartRateZones in your experience?
    • Can I rely on the array size of the result and remove my size checks in your experience?
  • 6 months ago in reply to Michael

    What do you do with T? Aren't you display it's "value"? Why can't T have a getValue() that return a String (or even String or Number or ...) and you display that?

  • 6 months ago in reply to flocsy🤠

    My data instance knows the region it is allowed to draw at (and also its "type" like e.g. arc area, small region, large region) and it knows its data and it knows the type of T - with this information my watchface can simple call data.draw(region, type, ...) for each data it wants to display and the data instance knows what to do... It may draw a progress bar, it may draw an icon and a text, it may draw a text only, it may apply a custom color or not, it may draw a graph... if everything can be reduced to returning a string or plain simple data, I could refactor all my logic to use a non generic base class and simple implement a plain interface instead...

    The result is following:

    In my watchface I simply load what the user selected to show. Then I call load on each item (or wait for the loaded data from a complication, the class handles that for me). Then I simple iterate over each data and draw it at its correct place. And I'm done. Extensibility is the key reason here: if I add a new data type, I simple implement a simple class and am done. Even if I add something completely know like a progress circle, I can simple implement a few lines of code in my data instance and the watchface code does not need to know anything about that...

  • 6 months ago in reply to Michael

    I'm telling the obvious that you still don't realize: your value is not an Object, it's a T. T is an interface, that has draw(region, type, ...) method, that each implementation implements... Once you fix this in your code you'll be able to properly type the parameter that you pass, and then inside that function call draw without the (incorrect) casting.

  • 6 months ago in reply to flocsy🤠

    The data has the draw functions, not T. T is just used to calculate the data... Generics can't simply be replaced like you say. Nevertheless, my problem is already solved and I may not have explained why I need the cast well enough and did not know I have to, but if you think about generics and why they exist, it should be obvious that there are use cases where you need them for type safety and in monkeyc you can only define base classes as non generics so each implementation must cast to T itself (and each implementation knows possible types of T)

  • 6 months ago in reply to Michael

    OK, I might have lost you in the process... But to me it sounds like the way you split code and data might be possible to juggle around a bit and that the "data", the data part of T go together with the code that needs to operate on them and is different between different implementations of T. But I don't see all the structure, so maybe I over simplify it or misunderstood it.

  • 6 months ago in reply to Michael
    Michael said:
    • Is it now save to remove the null check for the return value of UserProfile.getHeartRateZones in your experience?
    • Can I rely on the array size of the result and remove my size checks in your experience?

    Well...I have a few apps that call getHeartRateZones() with no null or size check. In a small test app, I tried calling UserProfile.getHeartRateZones(UserProfile.HR_ZONE_SPORT_SWIMMING) on a real device (FR955) that has no sport-specific HR zones, and it just returned the generic zones.

    So I assume the function is designed to always return valid data, which matches my recollection and the documentation (well, as far as it goes - obviously the doc doesn't specify the length of the returned array or explicitly say that you'll always get valid data).

    I will say that one of my apps has a consistent crash around zones-related code :/. So take what I said with a grain of salt haha

    I also found the following code in the source of a CIQ app that has 10k+ downloads (not like it proves much)

    [https://github.com/andan67/wormnav/blob/3325a44362e70015443b43c8412779e1a2802e6d/connectiq/source/Data.mc#L23-L25]

        function setMaxHeartRate() {
            maxHeartRate = UserProfile.getHeartRateZones(UserProfile.getCurrentSport())[5];
        }

    I did see a watchface with 50k+ downloads with a null check (but no size check). Not sure if this is due to a defensive coding or based on a real situation where the function returns null:

    [https://github.com/fevieira27/MoveToBeActive/blob/e6671194ed95df27fc019094b1f255779d36b84e/source/MtbA_functions.mc#L650-L685]

    I guess I would say that if you have the memory to spare, a null check probably wouldn't hurt, but a size check might be overkill. Most of my apps are data fields were originally designed for devices with very low amounts of available memory for CIQ data fields, so I obsessed over avoiding as much "unnecessary" code as possible in an attempt to reduce memory usage. But if you have enough memory, I don't think it hurts to code defensively.

  • 6 months ago in reply to FlowState

    And ofc there's the bug I linked that was posted a few years ago, which says getHeartRateZones() returns null when an activity hasn't been started. Who knows if it was fixed and/or it only applied to those old devices though.

    Ofc that would be a bug on Garmin's end, but it wouldn't hurt if we write our apps to mitigate/avoid bugs in the system that we can't control.