Merge branch 'InfiniTimeOrg:develop' into dice

.clang-format

@@ -59,6 +59,8 @@ Cpp11BracedListStyle: true
 DeriveLineEnding: false
 DerivePointerAlignment: false
 DisableFormat: false
+EmptyLineAfterAccessModifier: Never
+EmptyLineBeforeAccessModifier: LogicalBlock
 ExperimentalAutoDetectBinPacking: true
 FixNamespaceComments: false
 ForEachMacros:
@@ -78,6 +80,7 @@ IncludeCategories:
     SortPriority: 0
 IncludeIsMainRegex: '(Test)?$'
 IncludeIsMainSourceRegex: ''
+IndentAccessModifiers: false
 IndentCaseLabels: true
 IndentGotoLabels: true
 IndentPPDirectives: BeforeHash
@@ -92,6 +95,7 @@ MacroBlockBegin: ''
 MacroBlockEnd: ''
 MaxEmptyLinesToKeep: 1
 NamespaceIndentation: All
+PackConstructorInitializers: NextLine
 PenaltyBreakAssignment: 2
 PenaltyBreakBeforeFirstCallParameter: 19
 PenaltyBreakComment: 300
@@ -101,13 +105,16 @@ PenaltyBreakTemplateDeclaration: 10
 PenaltyExcessCharacter: 1000000
 PenaltyReturnTypeOnItsOwnLine: 60
 PointerAlignment: Left
+ReferenceAlignment: Pointer
 ReflowComments: true
+SeparateDefinitionBlocks: Always
 SortIncludes: false
 SortUsingDeclarations: true
 SpaceAfterCStyleCast: true
 SpaceAfterLogicalNot: false
 SpaceAfterTemplateKeyword: true
 SpaceBeforeAssignmentOperators: true
+SpaceBeforeCaseColon: false
 SpaceBeforeCpp11BracedList: true
 SpaceBeforeCtorInitializerColon: true
 SpaceBeforeInheritanceColon: true
@@ -123,7 +130,7 @@ SpacesInCStyleCastParentheses: false
 SpacesInParentheses: false
 SpacesInSquareBrackets: false
 SpaceBeforeSquareBrackets: false
-# Needs new Clang: SpaceAroundPointerQualifiers: After
+SpaceAroundPointerQualifiers: Default
 Standard: Latest
 StatementMacros:
   - Q_UNUSED

.github/workflows/format.yml

@@ -23,7 +23,7 @@ jobs:
       - name: Install clang-format
         run: |
           sudo apt-get update
-          sudo apt-get -y install clang-format-12
+          sudo apt-get -y install clang-format-14
 
       - name: Check formatting
         run: tests/test-format.sh
@@ -55,7 +55,7 @@ jobs:
     - name: Install clang-tidy
       run: |
         apt-get update
-        apt-get -y install clang-tidy-12
+        apt-get -y install clang-tidy-14
     - name: Prepare environment
       shell: bash
       env:

.github/workflows/getSize.sh

@@ -0,0 +1,19 @@
+#!/bin/sh
+
+# Requires environment variables from docker/build.sh
+
+set -e
+
+SIZE_BIN="$TOOLS_DIR/$GCC_ARM_PATH/bin/arm-none-eabi-size"
+[ ! -x "$SIZE_BIN" ] && exit 1
+
+[ -z "$1" ] && exit 1
+SIZE_OUTPUT=$($SIZE_BIN "$1" | tail -n1)
+
+TEXT_SIZE=$(echo "$SIZE_OUTPUT" | cut -f 1 |tr -d '[:blank:]')
+DATA_SIZE=$(echo "$SIZE_OUTPUT" | cut -f 2 |tr -d '[:blank:]')
+BSS_SIZE=$(echo "$SIZE_OUTPUT" | cut -f 3 |tr -d '[:blank:]')
+
+echo "text_size=$TEXT_SIZE"
+echo "data_size=$DATA_SIZE"
+echo "bss_size=$BSS_SIZE"

.github/workflows/main.yml

@@ -13,28 +13,32 @@ on:
       - 'doc/**'
       - '**.md'
 
-
 jobs:
   build-firmware:
     runs-on: ubuntu-latest
     container:
       image: infinitime/infinitime-build
+    outputs:
+      text_size: ${{ steps.output-sizes.outputs.text_size }}
+      data_size: ${{ steps.output-sizes.outputs.data_size }}
+      bss_size: ${{ steps.output-sizes.outputs.bss_size }}
+    env:
+      # InfiniTime sources are downloaded to the current directory.
+      # Override SOURCES_DIR in build.sh
+      SOURCES_DIR: .
     steps:
-      # This workaround fixes the error "unsafe repository (REPO is owned by someone else)".
-      # See https://github.com/actions/checkout/issues/760 and https://github.com/actions/checkout/issues/766
-      # The fix in "actions/checkout@v2" was not sufficient as the build process also uses git (to get the current
-      # commit hash, for example).
-      - name: Workaround permission issues
-        run: git config --global --add safe.directory "$GITHUB_WORKSPACE"
       - name: Checkout source files
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with:
           submodules: recursive
       - name: Build
         shell: bash
-        env:
-          SOURCES_DIR: .
         run: /opt/build.sh all
+      - name: Output build size
+        id: output-sizes
+        run: |
+          . /opt/build.sh
+          .github/workflows/getSize.sh "$BUILD_DIR"/src/pinetime-app-*.out >> $GITHUB_OUTPUT
       # Unzip the package because Upload Artifact will zip up the files
       - name: Unzip DFU package
         run: unzip ./build/output/pinetime-mcuboot-app-dfu-*.zip -d ./build/output/pinetime-mcuboot-app-dfu
@@ -93,3 +97,105 @@ jobs:
       with:
         name: infinisim-${{ github.head_ref }}
         path: build_lv_sim/infinisim
+
+  get-base-ref-size:
+    if: github.event_name == 'pull_request'
+    runs-on: ubuntu-22.04
+    container:
+      image: infinitime/infinitime-build
+    outputs:
+      text_size: ${{ steps.output-sizes.outputs.text_size }}
+      data_size: ${{ steps.output-sizes.outputs.data_size }}
+      bss_size: ${{ steps.output-sizes.outputs.bss_size }}
+    env:
+      # InfiniTime sources are downloaded to the current directory.
+      # Override SOURCES_DIR in build.sh
+      SOURCES_DIR: .
+    steps:
+    - name: Cache sources
+      id: cache-sources
+      uses: actions/cache@v3
+      with:
+        path: .
+        key: source-files-${{ github.event.pull_request.base.sha }}
+
+    - if: ${{ steps.cache-sources.outputs.cache-hit != 'true' }}
+      name: Checkout source files
+      uses: actions/checkout@v3
+      with:
+        ref: ${{ github.base_ref }}
+        submodules: recursive
+
+    - if: ${{ steps.cache-sources.outputs.cache-hit != 'true' }}
+      name: Build
+      shell: bash
+      # Only pinetime-app target is needed, but post_build.sh fails
+      run: /opt/build.sh all
+
+    - name: Output build size
+      id: output-sizes
+      run: |
+        . /opt/build.sh
+        .github/workflows/getSize.sh "$BUILD_DIR"/src/pinetime-app-*.out >> $GITHUB_OUTPUT
+
+  compare-build-size:
+    if: github.event_name == 'pull_request'
+    needs: [build-firmware, get-base-ref-size]
+    runs-on: ubuntu-latest
+    steps:
+    - name: Compare build size
+      id: output-sizes-diff
+      run: |
+        TEXT_SIZE=${{ needs.build-firmware.outputs.text_size }}
+        DATA_SIZE=${{ needs.build-firmware.outputs.data_size }}
+        BSS_SIZE=${{ needs.build-firmware.outputs.bss_size }}
+
+        echo "text_size=$TEXT_SIZE"
+        echo "data_size=$DATA_SIZE"
+        echo "bss_size=$BSS_SIZE"
+
+        echo "text_size=$TEXT_SIZE" >> $GITHUB_OUTPUT
+        echo "data_size=$DATA_SIZE" >> $GITHUB_OUTPUT
+        echo "bss_size=$BSS_SIZE" >> $GITHUB_OUTPUT
+
+        TEXT_SIZE_BASE=${{ needs.get-base-ref-size.outputs.text_size }}
+        DATA_SIZE_BASE=${{ needs.get-base-ref-size.outputs.data_size }}
+        BSS_SIZE_BASE=${{ needs.get-base-ref-size.outputs.bss_size }}
+
+        TEXT_SIZE_DIFF=$((TEXT_SIZE - TEXT_SIZE_BASE))
+        DATA_SIZE_DIFF=$((DATA_SIZE - DATA_SIZE_BASE))
+        BSS_SIZE_DIFF=$((BSS_SIZE - BSS_SIZE_BASE))
+
+        echo "text_diff=$TEXT_SIZE_DIFF"
+        echo "data_diff=$DATA_SIZE_DIFF"
+        echo "bss_diff=$BSS_SIZE_DIFF"
+
+        echo "text_diff=$TEXT_SIZE_DIFF" >> $GITHUB_OUTPUT
+        echo "data_diff=$DATA_SIZE_DIFF" >> $GITHUB_OUTPUT
+        echo "bss_diff=$BSS_SIZE_DIFF" >> $GITHUB_OUTPUT
+
+    - name: Find Comment
+      # Due to a security concern, comments can only be created in the context of branches in the repo.
+      # PRs from forks can't get the comment.
+      if: github.event.pull_request.head.repo.full_name == github.repository
+      uses: peter-evans/find-comment@v2
+      id: build-size-comment
+      with:
+        issue-number: ${{ github.event.pull_request.number }}
+        comment-author: 'github-actions[bot]'
+        body-includes: Build size and comparison to
+
+    - name: Create or update comment
+      if: github.event.pull_request.head.repo.full_name == github.repository
+      uses: peter-evans/create-or-update-comment@v2
+      with:
+        comment-id: ${{ steps.build-size-comment.outputs.comment-id }}
+        issue-number: ${{ github.event.pull_request.number }}
+        body: |
+          Build size and comparison to ${{ github.base_ref }}:
+          | Section | Size | Difference |
+          | ------- | ---- | ---------- |
+          | text    | ${{ needs.build-firmware.outputs.text_size }}B | ${{ steps.output-sizes-diff.outputs.text_diff }}B |
+          | data    | ${{ needs.build-firmware.outputs.data_size }}B | ${{ steps.output-sizes-diff.outputs.data_diff }}B |
+          | bss     | ${{ needs.build-firmware.outputs.bss_size }}B  | ${{ steps.output-sizes-diff.outputs.bss_diff }}B  |
+        edit-mode: replace

docker/build.sh

@@ -20,7 +20,7 @@ export GCC_ARM_VER=${GCC_ARM_VER:="10.3-2021.10"}
 export NRF_SDK_VER=${NRF_SDK_VER:="nRF5_SDK_15.3.0_59ac345"}
 
 MACHINE="$(uname -m)"
-[[ "$MACHINE" == "arm64" ]] && MACHINE="aarch64"
+[ "$MACHINE" = "arm64" ] && MACHINE="aarch64"
 
 export GCC_ARM_PATH="gcc-arm-none-eabi-$GCC_ARM_VER"
 
@@ -29,9 +29,9 @@ main() {
 
   mkdir -p "$TOOLS_DIR"
 
-  [[ ! -d "$TOOLS_DIR/$GCC_ARM_PATH" ]] && GetGcc
-  [[ ! -d "$TOOLS_DIR/$NRF_SDK_VER" ]] && GetNrfSdk
-  [[ ! -d "$TOOLS_DIR/mcuboot" ]] && GetMcuBoot
+  [ ! -d "$TOOLS_DIR/$GCC_ARM_PATH" ] && GetGcc
+  [ ! -d "$TOOLS_DIR/$NRF_SDK_VER" ] && GetNrfSdk
+  [ ! -d "$TOOLS_DIR/mcuboot" ] && GetMcuBoot
 
   mkdir -p "$BUILD_DIR"
 
@@ -73,13 +73,13 @@ CmakeGenerate() {
 
 CmakeBuild() {
   local target="$1"
-  [[ -n "$target" ]] && target="--target $target"
-  if cmake --build "$BUILD_DIR" --config $BUILD_TYPE $target -- -j$(nproc)
-    then return 0; else return 1; 
-  fi
+  [ -n "$target" ] && target="--target $target"
+  cmake --build "$BUILD_DIR" --config $BUILD_TYPE $target -- -j$(nproc)
+  BUILD_RESULT=$?
+  return $BUILD_RESULT
 }
 
-if [[ $SOURCED == "false" ]]; then
+if [ $SOURCED = "false" ]; then
   # It is important to return exit code of main
   # To be future-proof, this is handled explicitely
   main "$@"