Under Review
over 2 years ago

bug: compiler 4.1.6 included broken parts of the 4.2.0 Beta 1 with -l 3

I just found that sdk 4.1.6 included parts of 4.2.0 Beta 1 that other bugreports reported recently as breaking some apps. More interestingly this inclusion wasn't mentioned in the announcment on 4.1.6.

See: https://forums.garmin.com/developer/connect-iq/i/bug-reports/sdk-4-2-0-beta-1-won-t-compile-appbase-getproperty

import Toybox.Application;
import Toybox.Application.Properties;

function setConfig(key as PropertyKeyType, val as PropertyValueType) as Void {
    MyApp.setProperty(key, val);
}

java -Xms1g -Dfile.encoding=UTF-8 -Dapple.awt.UIElement=true -jar /home/gavriel/Garmin/ConnectIQ/Sdks/connectiq-sdk-lin-4.1.6-2022-11-04-17f8cfdf3/bin/monkeybrains.jar -o bin/MyApp.prg -f /home/gavriel/Garmin/MyApp/monkey.jungle;/home/gavriel/Garmin/MyApp/barrels.jungle -y /home/gavriel/Garmin/developer_key.der -d fr235_sim -w -l 3

ERROR: fr235: /home/gavriel/Garmin/MyApp/source/Config.mc:17,4: '$.Toybox.Application.AppBase.setProperty' is deprecated.

flocsy🤠
  • over 1 year ago

    And if for whatever reason you really want to get rid of geproperty loigic surely there must be a way that it's not frustrating for anyone. Why not have the wrapper functionaliy inside the virtual machine?

    Like you pointed out elsewhere, the semantics/behavior of the new API aren't the same as the old API, so would it be appropriate for the VM to silently substitute new for old?

  • over 1 year ago

    From a refactoring standpoint it might have seemed like the best option to split Properties from Storage, but this was a big mistake from a backwards compatibilty poiint of view.

    It's simply not rhe right choice for something which is as widely used as AppBase.getProperty/setProperty. 

    I (still) don't think it's a good option to force a wrapper onto the developer or force the developer to use excludedannotations. It unnecessarily litters code.

    Mistakes and frustrations aside, I also don't see why it is such a big problem to keep the api around? What's the big deal....???

    And if for whatever reason you really want to get rid of geproperty loigic surely there must be a way that it's not frustrating for anyone. Why not have the wrapper functionaliy inside the virtual machine?

    Or alternatively it might even be easier to do the remapping while you are assembling the different prg files. Just write other opcodes/instructions while assembling the prg depending on the device api level? This doesn't sound like a hard thing to do.   

  • over 1 year ago in reply to FlowState

    Well, just wrote out a nice long post about this only to have the forum software eat it after I clicked reply. This is going to be a quick and dirty version of that post.

    Yes, we are intend to maintain compatibility. If a feature is available on day 1 device firmware, we should not remove it from that device. We have made mistakes in this area, but our general rule it to avoid breaking apps.


  • over 1 year ago

    We aren’t talking about removing functionality from existing devices here. Just removing from the new ones where usable alternatives exist.

    Devices that have the Storage or Properties modules should use them. Devices that do not can continue to use the AppBase.getProperty and .setProperty methods.

    Sure, that's always been my understanding. But at least at the time of those bug report, people were saying that you still get the warning if you call AppBase.getProperty() or AppBase.setProperty() in code that's built for an older device, and that it's an error in strict type-checking mode. That's one of the complaints here (the other is that, according to some, getProperty() and setProperty() have advantages that the new API does not.)

    I tried compiling the following line with SDK 6.2.2, fr235 and I still get the warning:

    AppBase.getProperty("foo");
    However, it's always a warning, even in strict type-checking mode. So that eliminates the main problem in the OP where you can't enable strict type-checking without breaking apps that need to use getProperty/setProperty (for old devices.)

    If using a recent compiler version, we have branch elimination optimizations that can be used to write code that is essentially a compile-time branch.

    I assume you mean something like this:

    if (AppBase has :getProperty) {
      val = AppBase.getProperty("foo");
    } else {
      // ...
    }

    Anyway, I realize that it would be very difficult to have the warning work in such way that only invalid uses of getProperty/setProperty() are flagged, so keeping it as a warning at all type check levels seems like a good compromise to me. Thanks for doing that!

    Every time we try to remove these functions for a new device, we are rebuffed because it will break someone’s app.

    Yeah, I can see how that would be frustrating. Does that imply that these functions would never be removed for existing devices which already have them? I'm not asking to be difficult or game the system, but only because I have some apps which aren't currently maintained, and I'm curious if they could break in the future due to device updates.

  • over 1 year ago in reply to FlowState

    We aren’t talking about removing functionality from existing devices here. Just removing from the new ones where usable alternatives exist.

    Devices that have the Storage or Properties modules should use them. Devices that do not can continue to use the AppBase.getProperty and .setProperty methods.

    That said, we provide at least two ways to do this. We have had support conditional compilation using jungle files and annotations for some time. If using a recent compiler version, we have branch elimination optimizations that can be used to write code that is essentially a compile-time branch.

    I can post examples later. I’m on mobile right now and getting ready to go for a mountain bike ride.

    I’m sorry to be venting here, but we are seeing very popular apps by competent developers that are still using these deprecated functions. Every time we try to remove these functions for a new device, we are rebuffed because it will break someone’s app.

    It is extremely frustrating.