Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "zero-length board ID" rule #160

Open
jpconstantineau opened this issue Jan 24, 2021 · 4 comments
Open

Add "zero-length board ID" rule #160

jpconstantineau opened this issue Jan 24, 2021 · 4 comments
Assignees
Labels
criticality: medium Of moderate impact topic: code Related to content of the project itself type: enhancement Proposed improvement

Comments

@jpconstantineau
Copy link

jpconstantineau commented Jan 24, 2021

I am running the linter using your github-action (thanks for making it available) and I am faced with these three rules failing and it seems that everything is defined as per the specification. It does load fine in the IDE. (two of them are errors, one is a warning)

For reference, here is the link to my boards.txt. It's based on Adafruit's nRF52 Feather BSP.
Here is the github action output. It does not provide any hint which board is causing the problem.

Are the identifiers too long? Are they too short? Is it a rule parsing problem? is it a problem with the rule or my file?

Here is a short section of my board.txt file (1 out of 9 boards):

# ----------------------------------
# 4x4 macropad nRF52832
# ----------------------------------

4x4macropad_nrf52832.name=4x4 Macropad (nRF52832)
4x4macropad_nrf52832.bootloader.tool=bootburn

# Upload
4x4macropad_nrf52832.upload.tool=nrfutil
4x4macropad_nrf52832.upload.protocol=nrfutil
4x4macropad_nrf52832.upload.use_1200bps_touch=false
4x4macropad_nrf52832.upload.wait_for_upload_port=false
4x4macropad_nrf52832.upload.native_usb=false
4x4macropad_nrf52832.upload.maximum_size=290816
4x4macropad_nrf52832.upload.maximum_data_size=52224

# Build
4x4macropad_nrf52832.build.mcu=cortex-m4
4x4macropad_nrf52832.build.f_cpu=64000000
4x4macropad_nrf52832.build.board=4X4MACROPAD_NRF52832
4x4macropad_nrf52832.build.core=nRF5
4x4macropad_nrf52832.build.variant=4x4macropad_nrf52832
4x4macropad_nrf52832.build.usb_manufacturer="BlueMicro"
4x4macropad_nrf52832.build.usb_product="4x4 Macropad nRF52832"
4x4macropad_nrf52832.build.extra_flags=-DNRF52832_XXAA -DNRF52
4x4macropad_nrf52832.build.ldscript=nrf52832_s132_v6.ld

# SofDevice Menu
4x4macropad_nrf52832.menu.softdevice.s132v6=0.4.0 SoftDevice s132 6.1.1
4x4macropad_nrf52832.menu.softdevice.s132v6.build.sd_name=s132
4x4macropad_nrf52832.menu.softdevice.s132v6.build.bl_version=0.4.0
4x4macropad_nrf52832.menu.softdevice.s132v6.build.sd_version=6.1.1
4x4macropad_nrf52832.menu.softdevice.s132v6.build.sd_fwid=0x00B7


# Debug Menu
4x4macropad_nrf52832.menu.debug.l0=Level 0 (Release)
4x4macropad_nrf52832.menu.debug.l0.build.debug_flags=-DCFG_DEBUG=0
4x4macropad_nrf52832.menu.debug.l1=Level 1 (Error Message)
4x4macropad_nrf52832.menu.debug.l1.build.debug_flags=-DCFG_DEBUG=1
4x4macropad_nrf52832.menu.debug.l2=Level 2 (Full Debug)
4x4macropad_nrf52832.menu.debug.l2.build.debug_flags=-DCFG_DEBUG=2
4x4macropad_nrf52832.menu.debug.l3=Level 3 (Segger SystemView)
4x4macropad_nrf52832.menu.debug.l3.build.debug_flags=-DCFG_DEBUG=3
4x4macropad_nrf52832.menu.debug.l3.build.sysview_flags=-DCFG_SYSVIEW=1
@jpconstantineau jpconstantineau changed the title Rules PF005, PF007 and PF016 failed even though all boards in boards.txt have *.build.core, *.build.board and *.upload.tool are all defined Rules PF005, PF007 and PF016 failed even though all boards in boards.txt have *.build.core, *.build.board and *.upload.tool all defined Jan 24, 2021
@per1234
Copy link
Contributor

per1234 commented Jan 24, 2021

Hi @jpconstantineau.

I am running the linter using your github-action (thanks for making it available)

You're welcome. I'm glad if you find it useful.

Are the identifiers too long? Are they too short? Is it a rule parsing problem? is it a problem with the rule or my file?

The problem is here:
https://github.com/jpconstantineau/Community_nRF52_Arduino/blob/4dacbfabb170843c275c4afaac6f8e2c08d1b319/boards.txt#L361

=======

boards.txt uses a "key=value" format. Apparently zero length keys are supported by the format. So what this line does is create a board ID that has zero length. This board is missing the required properties. So this is the reason why the error/warning messages only show an empty space where you would expect the board ID to be.

The fix would be to either remove line 361 from your boards.txt or else change it to a comment by prefixing it with #.

This is certainly a confusing error to troubleshoot (though likely a rare occurrence). I guess the experience would be improved by adding a "zero-length board ID" rule to Arduino Lint.

@jpconstantineau
Copy link
Author

Thank you. This fixed my issue with my boards.txt file. I had the offending line in 2 locations.

Indeed, a new rule for zero-length board ID would be useful. I really don't know why I had this type of separator when everything else was commented. I assume there is a rule to find non-commented, non-"key-value" formatted lines but this specific line would not have triggered it.

I am not sure if the linting framework allows for giving line numbers but that would be useful too.

I won't close the issue as you may want to link a new rule PR to this issue. I'll let you decide. Thanks

@per1234
Copy link
Contributor

per1234 commented Jan 24, 2021

I assume there is a rule to find non-commented, non-"key-value" formatted lines but this specific line would not have triggered it.

Yes, PF002 check for invalid data format. It's just that your problematic lines happened to have a completely valid data format as far as the Arduino "properties" data format is concerned. In the case of invalid data format, the error message is much more helpful:

Rule PF002 result: fail
ERROR: boards.txt has an invalid format: Error reading file: Error parsing data at line 42: Invalid line format, should be 'key=value'

I am not sure if the linting framework allows for giving line numbers but that would be useful too.

The problem is that the configuration files are parsed into a data map (as shown below) before being validated, so the identification of the JSON schema violation is a somewhat cryptic JSON pointer to the path in that data structure. So it would be necessary to backtrack from that pointer to the location in the source file. And of course in the case of rules about required or recommended properties, such as the ones mentioned in this issue, a line number is not applicable.

{
  "4x4macropad_nrf52832": {
    "bootloader.tool": "bootburn",
    "build.board": "4X4MACROPAD_NRF52832",
    "build.core": "nRF5",
    "build.extra_flags": "-DNRF52832_XXAA -DNRF52",
    "build.f_cpu": "64000000",
    "build.ldscript": "nrf52832_s132_v6.ld",
    "build.mcu": "cortex-m4",
    "build.usb_manufacturer": "\"BlueMicro\"",
    "build.usb_product": "\"4x4 Macropad nRF52832\"",
    "build.variant": "4x4macropad_nrf52832",
    "menu": {
      "debug": {
        "l0": {
          "build.debug_flags": "-DCFG_DEBUG=0"
        },
        "l1": {
          "build.debug_flags": "-DCFG_DEBUG=1"
        },
        "l2": {
          "build.debug_flags": "-DCFG_DEBUG=2"
        },
        "l3": {
          "build.debug_flags": "-DCFG_DEBUG=3",
          "build.sysview_flags": "-DCFG_SYSVIEW=1"
        }
      },
      "softdevice": {
        "s132v6": {
          "build.bl_version": "0.4.0",
          "build.sd_fwid": "0x00B7",
          "build.sd_name": "s132",
          "build.sd_version": "6.1.1"
        }
      }
    },
    "name": "4x4 Macropad (nRF52832)",
    "upload.maximum_data_size": "52224",
    "upload.maximum_size": "290816",
    "upload.native_usb": "false",
    "upload.protocol": "nrfutil",
    "upload.tool": "nrfutil",
    "upload.use_1200bps_touch": "false",
    "upload.wait_for_upload_port": "false"
  },

... and so on

@jpconstantineau
Copy link
Author

The file being parsed and converted into a json file/object indeed wouldn't be able to keep the line number of the original file.
This clarifies things a lot. Thanks

@per1234 per1234 changed the title Rules PF005, PF007 and PF016 failed even though all boards in boards.txt have *.build.core, *.build.board and *.upload.tool all defined Add "zero-length board ID" rule Feb 22, 2021
@per1234 per1234 added priority: medium Resolution is a medium priority topic: code Related to content of the project itself type: enhancement Proposed improvement labels Feb 22, 2021
@rsora rsora added criticality: medium Of moderate impact and removed priority: medium Resolution is a medium priority labels Nov 2, 2021
@per1234 per1234 self-assigned this Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
criticality: medium Of moderate impact topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

No branches or pull requests

3 participants