search

microsoft / pxt-microbit

A Blocks / JavaScript code editor for the micro:bit built on Microsoft MakeCode
https://makecode.microbit.org
Other
710 stars 586 forks source link

Setting constants in `pxt.json` with `yotta.config.codal` does not work #5352

Open microbit-carlos opened 12 months ago

microbit-carlos commented 12 months ago

Describe the bug From the documentation in https://github.com/microsoft/pxt/blob/d6fa4dffb4bf95eb7c5ab8f93323d64f240bc157/docs/extensions/pxt-json.md#setting-c-constants-for-dal-config---yotta this should work, but when tested it does not set up the macro in the CODAL builds:

    "yotta": {
        "config": {
            "codal": {
                "CODAL_RANDOM_MACRO_NAME": 1
            }
        }
    }

Adding the macros to yotta.config instead does work, but that adds them to both V1 and V2 builds.

    "yotta": {
        "config": {
            "CODAL_AND_DAL_RANDOM_MACRO_NAME": 1
        }
    }

To Reproduce

To test this I've created this MakeCode programme with a C++ file that prints macro values, based on this @martinwork's extension for the same purpose: https://github.com/martinwork/pxt-cpp/blob/master/ext.cpp

The project example can be imported from: https://github.com/microbit-carlos/makecode-macrotest

  1. Go to https://makecode.microbit.org/beta
  2. Click on the "Import" button, and "import from URL"
  3. Enter the https://github.com/microbit-carlos/makecode-macrotest URL
  4. Inspect the macros defined in pxt.json
  5. Compile the programme and flash the micro:bit
  6. Inspect the output printed to serial
    "yotta": {
        "config": {
            "codal": {
                "MICROBIT_BLE_UTILITY_SERVICE": 1,
                "MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1,
                "RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1
            },
            "microbit-dal": {
                "RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1
            },
            "RANDOM_MACRO_IN_YOTTA_CONFIG": 1
        }
    }
uBit.serial.send("MICROBIT_BLE_UTILITY_SERVICE_PAIRING     = " MACROTOSTRING(MICROBIT_BLE_UTILITY_SERVICE_PAIRING) "\n");
uBit.serial.send("MICROBIT_BLE_UTILITY_SERVICE             = " MACROTOSTRING(MICROBIT_BLE_UTILITY_SERVICE) "\n");
uBit.serial.send("RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL       = " MACROTOSTRING(RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL) "\n");
uBit.serial.send("RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL = " MACROTOSTRING(RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL) "\n");
uBit.serial.send("RANDOM_MACRO_IN_YOTTA_CONFIG             = " MACROTOSTRING(RANDOM_MACRO_IN_YOTTA_CONFIG) "\n");

Output:

MICROBIT_BLE_UTILITY_SERVICE_PAIRING     = 0
MICROBIT_BLE_UTILITY_SERVICE             = 0
RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL       = RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL
RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL = RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL
RANDOM_MACRO_IN_YOTTA_CONFIG             = 1

As show, RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL should be equal to 1, but as an undefined macro it prints its name. RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL is correctly undefined, as that's a macro for V1 DAL only.

Expected behavior

The expected output on a micro:bit V2 would have been:

MICROBIT_BLE_UTILITY_SERVICE_PAIRING     = 1
MICROBIT_BLE_UTILITY_SERVICE             = 1
RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL       = 1
RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL = RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL
RANDOM_MACRO_IN_YOTTA_CONFIG             = 1

Screenshots N/A

micro:bit version (please complete the following information):

V2.xx

Desktop (please complete the following information): N/A

Additional context

makecode.microbit.org version: 6.1.5 Microsoft MakeCode version: 9.1.11 microbit runtime version: v2.2.0-rc6 codal-microbit-v2 runtime version: v0.2.57

martinwork commented 12 months ago

Yes, CPP macro definitions inside "codal": {} don't seem to work for me.

What definitions are generated by the documented CODAL example? https://makecode.com/extensions/pxt-json#:~:text=microbit%2Ddal%3A-,CODAL%20example,-%3A CODAL needs DEVICE_COMPONENT_COUNT and DEVICE_DMESG_BUFFER_SIZE

Is there a scheme like the yotta DAL equivalent? "yotta": { "config": { "microbit-dal": { xxx-yyy }}} becomes YOTTA_CFG_MICROBIT_DAL_XXX_YYY and "yotta": { "config": { "codal": { xxx-yyy }}} becomes DEVICE_XXX_YYY, or perhaps YOTTA_CFG_CODAL_XXX_YYY

So does the example here generate DEVICE_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL or YOTTA_CFG_CODAL_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL?

When does yotta_cfg_mappings get used? https://github.com/lancaster-university/codal-microbit-v2/blob/master/inc/compat/yotta_cfg_mappings.h

microbit-carlos commented 12 months ago

Ah, yeah, looks like you are on the right track @martinwork.

With these values in a pxt.json file:

    "yotta": {
        "config": {
            "codal": {
                "MICROBIT_BLE_UTILITY_SERVICE": 1,
                "MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1,
                "RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1
            },
            "microbit-dal": {
                "RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1
            },
            "RANDOM_MACRO_IN_YOTTA_CONFIG": 1
        }
    }

With a local build I can inspect the generated codal.json file:

Click me to see the full json contents ```json { "target": { "name": "codal-microbit-v2", "url": "https://github.com/lancaster-university/codal-microbit-v2", "branch": "v0.2.57", "type": "git" }, "definitions": { "DEVICE_MICROBIT_BLE_UTILITY_SERVICE": 1, "DEVICE_MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1, "DEVICE_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1, "RANDOM_MACRO_IN_YOTTA_CONFIG": 1, "MICROBIT_DAL_STACK_SIZE": 1280, "MICROBIT_DAL_GATT_TABLE_SIZE": "0x700", "MICROBIT_DAL_BLUETOOTH_PRIVATE_ADDRESSING": 0, "MICROBIT_DAL_BLUETOOTH_ADVERTISING_TIMEOUT": 0, "MICROBIT_DAL_BLUETOOTH_TX_POWER": 6, "MICROBIT_DAL_BLUETOOTH_DFU_SERVICE": 1, "MICROBIT_DAL_BLUETOOTH_EVENT_SERVICE": 1, "MICROBIT_DAL_BLUETOOTH_DEVICE_INFO_SERVICE": 1, "MICROBIT_DAL_BLUETOOTH_EDDYSTONE_URL": 1, "MICROBIT_DAL_BLUETOOTH_EDDYSTONE_UID": 1, "MICROBIT_DAL_BLUETOOTH_OPEN": 0, "MICROBIT_DAL_BLUETOOTH_PAIRING_MODE": 1, "MICROBIT_DAL_BLUETOOTH_WHITELIST": 1, "MICROBIT_DAL_BLUETOOTH_SECURITY_LEVEL": "SECURITY_MODE_ENCRYPTION_NO_MITM", "MICROBIT_DAL_BLUETOOTH_PARTIAL_FLASHING": 1, "MICROBIT_DAL_BLUETOOTH_ENABLED": 1, "MICROBIT_DAL_FIBER_USER_DATA": 1, "MICROBIT_DAL_PXT": 1, "MICROBIT_DAL_RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1, "YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE": 1, "YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1, "YOTTA_CFG_CODAL_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1, "YOTTA_CFG_RANDOM_MACRO_IN_YOTTA_CONFIG": 1, "YOTTA_CFG_MICROBIT_DAL_STACK_SIZE": 1280, "YOTTA_CFG_MICROBIT_DAL_GATT_TABLE_SIZE": "0x700", "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_PRIVATE_ADDRESSING": 0, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_ADVERTISING_TIMEOUT": 0, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_TX_POWER": 6, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_DFU_SERVICE": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_EVENT_SERVICE": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_DEVICE_INFO_SERVICE": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_EDDYSTONE_URL": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_EDDYSTONE_UID": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_OPEN": 0, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_PAIRING_MODE": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_WHITELIST": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_SECURITY_LEVEL": "SECURITY_MODE_ENCRYPTION_NO_MITM", "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_PARTIAL_FLASHING": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_ENABLED": 1, "YOTTA_CFG_MICROBIT_DAL_FIBER_USER_DATA": 1, "YOTTA_CFG_MICROBIT_DAL_PXT": 1, "YOTTA_CFG_MICROBIT_DAL_RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1, "PXT_TARGET": "\"microbit\"" }, "config": { "DEVICE_MICROBIT_BLE_UTILITY_SERVICE": 1, "DEVICE_MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1, "DEVICE_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1, "RANDOM_MACRO_IN_YOTTA_CONFIG": 1, "MICROBIT_DAL_STACK_SIZE": 1280, "MICROBIT_DAL_GATT_TABLE_SIZE": "0x700", "MICROBIT_DAL_BLUETOOTH_PRIVATE_ADDRESSING": 0, "MICROBIT_DAL_BLUETOOTH_ADVERTISING_TIMEOUT": 0, "MICROBIT_DAL_BLUETOOTH_TX_POWER": 6, "MICROBIT_DAL_BLUETOOTH_DFU_SERVICE": 1, "MICROBIT_DAL_BLUETOOTH_EVENT_SERVICE": 1, "MICROBIT_DAL_BLUETOOTH_DEVICE_INFO_SERVICE": 1, "MICROBIT_DAL_BLUETOOTH_EDDYSTONE_URL": 1, "MICROBIT_DAL_BLUETOOTH_EDDYSTONE_UID": 1, "MICROBIT_DAL_BLUETOOTH_OPEN": 0, "MICROBIT_DAL_BLUETOOTH_PAIRING_MODE": 1, "MICROBIT_DAL_BLUETOOTH_WHITELIST": 1, "MICROBIT_DAL_BLUETOOTH_SECURITY_LEVEL": "SECURITY_MODE_ENCRYPTION_NO_MITM", "MICROBIT_DAL_BLUETOOTH_PARTIAL_FLASHING": 1, "MICROBIT_DAL_BLUETOOTH_ENABLED": 1, "MICROBIT_DAL_FIBER_USER_DATA": 1, "MICROBIT_DAL_PXT": 1, "MICROBIT_DAL_RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1, "YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE": 1, "YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1, "YOTTA_CFG_CODAL_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1, "YOTTA_CFG_RANDOM_MACRO_IN_YOTTA_CONFIG": 1, "YOTTA_CFG_MICROBIT_DAL_STACK_SIZE": 1280, "YOTTA_CFG_MICROBIT_DAL_GATT_TABLE_SIZE": "0x700", "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_PRIVATE_ADDRESSING": 0, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_ADVERTISING_TIMEOUT": 0, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_TX_POWER": 6, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_DFU_SERVICE": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_EVENT_SERVICE": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_DEVICE_INFO_SERVICE": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_EDDYSTONE_URL": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_EDDYSTONE_UID": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_OPEN": 0, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_PAIRING_MODE": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_WHITELIST": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_SECURITY_LEVEL": "SECURITY_MODE_ENCRYPTION_NO_MITM", "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_PARTIAL_FLASHING": 1, "YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_ENABLED": 1, "YOTTA_CFG_MICROBIT_DAL_FIBER_USER_DATA": 1, "YOTTA_CFG_MICROBIT_DAL_PXT": 1, "YOTTA_CFG_MICROBIT_DAL_RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1, "PXT_TARGET": "\"microbit\"" }, "application": "pxtapp", "output_folder": "build", "pxt_gitrepo": "lancaster-university/microbit-v2-samples", "pxt_gittag": "v0.2.13" } ```

The interesting bits being:

  "definitions": {
    "DEVICE_MICROBIT_BLE_UTILITY_SERVICE": 1,
    "DEVICE_MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1,
    "DEVICE_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1,
    "RANDOM_MACRO_IN_YOTTA_CONFIG": 1,
    "MICROBIT_DAL_RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1,
    "YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE": 1,
    "YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1,
    "YOTTA_CFG_CODAL_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1,
    "YOTTA_CFG_RANDOM_MACRO_IN_YOTTA_CONFIG": 1,
    "YOTTA_CFG_MICROBIT_DAL_RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1,
  },
  "config": {
    "DEVICE_MICROBIT_BLE_UTILITY_SERVICE": 1,
    "DEVICE_MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1,
    "DEVICE_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1,
    "RANDOM_MACRO_IN_YOTTA_CONFIG": 1,
    "MICROBIT_DAL_RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1,
    "YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE": 1,
    "YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE_PAIRING": 1,
    "YOTTA_CFG_CODAL_RANDOM_MACRO_IN_YOTTA_CONFIG_CODAL": 1,
    "YOTTA_CFG_RANDOM_MACRO_IN_YOTTA_CONFIG": 1,
    "YOTTA_CFG_MICROBIT_DAL_RANDOM_MACRO_IN_YOTTA_CONFIG_MICROBITDAL": 1,
  }

So MakeCode is prepending DEVICE_ and YOTTA_CFG_CODAL_ to the macros added via yotta.config.codal.

@abchatra we need a way to add C++ defines to a single target without MakeCode prepending to the macro name, so that we can add flags to V2 only with the data logging extension:

martinwork commented 12 months ago

@microbit-carlos We could add to yotta_cfg_mappings

#ifdef YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE 
    #define MICROBIT_BLE_UTILITY_SERVICE YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE 
#endif
#ifdef YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE_PAIRING 
    #define MICROBIT_BLE_UTILITY_SERVICE_PAIRING YOTTA_CFG_CODAL_MICROBIT_BLE_UTILITY_SERVICE_PAIRING 
#endif

It seems strange to be going via YOTTA, but at least the text in pxt.json would match the macro needed by the program, which stays MICROBIT rather than DEVICE

microbit-carlos commented 12 months ago

Yeah, I'd be a bit weird to be creating yotta macros for V2-only features.

And for any existing or future flags we'd have to manually "fix" each individual flag in the CODAL codebase, when it's something fixable in the MakeCode macro/codal.json generation. Basically we'd have to do the same thing for anything configurable in MicroBitConfig.h (and other header files) so that we could configure them only for V2. I'd prefer to resolve this in the MakeCode side.

I guess having "generic" CODAL defines automatically prepended with DEVICE_ might have been okay when originally implemented, as by convention that's how they've been named inside CODAL, however some since then some configuration macros have also been named starting with CODAL_ instead, so it no longer applies. And for micro:bit CODAL we do have a few configs that start with MICROBIT_ that are V2 specific.

So now, it would make sense for MakeCode to not prepend the macro names defined in yotta.config.codal. Or, at least, to also generate the defines without the prepend (like yotta.config does).

martinwork commented 12 months ago

Yes, I was only thinking about how to make the utility service defines work cleanly right now.

also generate the defines without the prepend

That sounds like it might be simple to do, as MakeCode already generates DEVICE_ and YOTTA_CFGCODAL versions.

Should there be some checks on the names and values, for example, is the name all capitals with a recognised prefix? I'm thinking yotta_cfg_mappings acts as a filter in yotta builds.

jaustin commented 11 months ago

After discussion in sync today @abchatra and team will take a look and come back to us early next week about whether we should deploy the workaround in the short term

abchatra commented 1 month ago

@carlosperate is this still an issue?

microbit-carlos commented 1 month ago

Yes, the issue being that MakeCode always prepends DEVICE_ to the name of every flag added to yotta.config.codal and MICROBIT_DAL to every flag added to yotta.config.microbit-dal.

At the moment we workaround this by either adding the same flag to both builds in yotta.config, or expecting the name mangling, but it is confusing.

The main problem can arise if somebody needs to create a flag for a C++ extension that needs to have different values in DAL vs CODAL. So a pxt.json file like this will end up with flags renamed to DEVICE_CUSTOM_FLAG in V2 and MICROBIT_DAL_CUSTOM_FLAG in V1:

"yotta": { 
    "config":  { 
        "codal": {
            "CUSTOM_FLAG": 1
        },
        "microbit-dal": {
            "CUSTOM_FLAG": 2
        }
    }
}