create_platform_config.py: Fix so it looks like other configs

[philipandag]

The configs were not working, the variables were not surrounded in "${" and "}", the values were scattered across different columns instead of aligned like in other configs.

[macpijan]

It works well now? Care to attach some example output in the comment, so we can make sure and potentially avoid merging something not working again?

[philipandag]

Certainly, I think I can paste it directly into a comment as it's not that long:

$ scripts/create_platform_config.py test-config
Creating new config test-config with includes []
New config file created at /home/fgolas/work/odroid-preparation/open-source-firmware-validation/scripts/../platform-configs/test-config.robot
Warning: 28 variables are not defined in any of the includes and need attention!

Contents of the new file at ./platform-configs/test-config.robot

*** Settings ***
Resource    include/default.robot


*** Variables ***
${INITIAL_DUT_CONNECTION_METHOD}=    ${TBD}
${DUT_CONNECTION_METHOD}=            ${TBD}
${FLASH_SIZE}=                       ${TBD}
${FLASH_LENGTH}=                     ${TBD}
${MANUFACTURER}=                     ${TBD}
${CPU}=                              ${TBD}
${POWER_CTRL}=                       ${TBD}
${FLASH_VERIFY_METHOD}=              ${TBD}
${WIFI_CARD}=                        ${TBD}
${MAX_CPU_TEMP}=                     ${TBD}
${FW_VERSION}=                       ${TBD}
${DMIDECODE_SERIAL_NUMBER}=          ${TBD}
${DMIDECODE_FIRMWARE_VERSION}=       ${TBD}
${DMIDECODE_PRODUCT_NAME}=           ${TBD}
${DMIDECODE_RELEASE_DATE}=           ${TBD}
${DMIDECODE_MANUFACTURER}=           ${TBD}
${DMIDECODE_VENDOR}=                 ${TBD}
${DMIDECODE_FAMILY}=                 ${TBD}
${DMIDECODE_TYPE}=                   ${TBD}
${DEVICE_USB_KEYBOARD}=              ${TBD}
${DEVICE_NVME_DISK}=                 ${TBD}
${DEVICE_AUDIO1}=                    ${TBD}
${DEVICE_AUDIO2}=                    ${TBD}
${DEVICE_AUDIO1_WIN}=                ${TBD}
${WIFI_CARD_UBUNTU}=                 ${TBD}
${USB_MODEL}=                        ${TBD}
${USB_DEVICE}=                       ${TBD}
${FLASHROM_FLAGS}=                   ${TBD}

Check if all lines containing '${TBD}' were moved to the new config

$ diff -b platform-configs/test-config.robot platform-configs/include/default.robot | grep '${TBD}'
> ${TBD}=                                             TBD_variable_not_set_03626549a59abf648ee59163b3b8acbf66c36513cb1e76d6e277bc044c926e30

The definition of ${TBD} should not be moved, so it works properly when it comes to exposing all ${TBD} variables.

When creating configs for the odroid-h4-plus platform I have noticed that probably not all variables which often need to be changed were set to ${EMPTY} before and so now are not set to ${TBD} and are not copied to new configs. It includes, for example ${TESTS_IN_FIRMWARE_SUPPORT} and ${TESTS_IN_UBUNTU_SUPPORT}. In default.robot they both are set to ${FALSE} which makes most tests unusable until you change any of them to ${TRUE}. It may be that there are more variables like that which could use some more attention when creating new configs.

[philipandag]

This could still be a bit improved because now all the parsing is performed manually and probably it could be made more straightforward and compatible by using robot framework libraries.

[mkopec]

@philipandag should this still be a draft?

[philipandag]

@philipandag should this still be a draft?

I think it should. Thre previous comment explains that. It might work unexpectedly because it does not use robot framework to read the configs and so it might break reading proper robot syntax. For example something like:

${VAR}=    
...    value

would 100% not work and the variable would be dropped.

If that's fine then I think there is nothing more to add at this moment. Ideally this script should be somehow joined with the get-robot-variables.sh script which we run on the DUT

[mkopec]

Ah okay I see the discussion.

How about running robotidy at the end of the script instead? The same robotidy we use in pre-commit already

[philipandag]

If you think it's fine then it can be merged. The idea to use some parsers from robot framework is the only reason this PR is in Draft. Connecting the two scripts should probably be done as a seperate task, if at all.

[philipandag]

How about running robotidy at the end of the script instead? The same robotidy we use in pre-commit already

It's not what this discussion is about. The problem is that right now the process of reading, processing & writing the configs is written by me by hand and supports only the basic syntax of config files. It might cause weird behaviors where robot reads some config fine but the script skips some variables.

This also adds unnecessary complexity to the script, which could otherwise be much simpler.

It works most of the time but sometimes it might behave unexpectedly and is a bit redundant because robot framework already does the work of processing robot framework files.

Running robotidy would not help with reading the configs, it might even break it sometimes. For example in the default.robot the variable ${TBD} has a pretty long value and because of that robotidy breaks it's definition into two lines which this script would not interpret properly. Right now it's simply skipped. In this single case it's not a problem as this variable should be skipped.