pez-globo / pufferfish-software

All software for the Pufferfish ventilator.
Apache License 2.0
0 stars 1 forks source link

Fix modernize-make-unique/shared IncludeStyle options #244

Closed ethanjli closed 3 years ago

ethanjli commented 3 years ago

This PR:

ethanjli commented 3 years ago

@akdhawan Can you try running your local installation of clang-tidy (version 12) on this branch and report what errors (if any) clang-tidy generates?

akdhawan commented 3 years ago

@ethanjli After running on my local installation of clang-tidy(version 11) on this branch, I see the following output documented in the output.log output.log

ethanjli commented 3 years ago

The cppcoreguidelines-pro-type-vararg complaints should be resolvable by modifying the implementation to use array indices rather than deferencing pointers in an iterator style. With only approx. two exceptions (namely the BufferedUART objects which need to be globals so that they can be referenced in the STM32CubeMX-generated file for interrupt handlers), the cppcoreguidelines-avoid-non-const-global-variables complaints should be resolvable by instantiating everything inside a section in the main function (I'll think more about whether we want to do this tomorrow, but https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global makes a reasonable argument about initialization orders).

ethanjli commented 3 years ago

It turns out that actually cppcoreguidelines-pro-type-vararg complaints were a false positive, and we actually do need to work with pointers in an iterator style (because of the C++ std library functions we're using). I've suppressed those complaints. I've also moved non-const global variable declarations into the main function wherever possible. I ran a quick test uploading this new version of the firmware to my Nucleo board (with the FDO2 driver commented out, since my FDO2 sensor is broken) and it successfully completed setup of the other sensors and was able to send flow measurements to the backend.

@akdhawan Can you try running clang-format and clang-tidy again and report what it outputs? If there are no errors, please go ahead and do an approval through the Github PR's code review interface, and then I'll merge it in.

For records-keeping:

  1. This project is licensed under Apache License v2.0 for any software, and Solderpad Hardware License v2.1 for any hardware - do you agree that your contributions to this project will be under these licenses, too? Yes
  2. Were any of these contributions also part of work you did for an employer or a client? No
  3. Does this work include, or is it based on, any third-party work which you did not create? No
ethanjli commented 3 years ago

I am going to close this PR for now - we can reopen it later as needed.