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

🔨 (version): Move firmware version from build_firmware script to Makefile #1023

Merged
merged 2 commits into from
Sep 22, 2022

Conversation

YannLocatelli
Copy link
Member

@YannLocatelli YannLocatelli commented Sep 14, 2022

  • Validation on application-signed file + robot with github CI on 20/09/2022

Add regex to check format of version

Suggest that build come in another PR

@YannLocatelli YannLocatelli added the 01 - type: task Something to do label Sep 14, 2022
@YannLocatelli YannLocatelli added this to the Next Release milestone Sep 14, 2022
@YannLocatelli YannLocatelli self-assigned this Sep 14, 2022
@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #1023 (c24af36) into develop (3e90d9b) will not change coverage.
The diff coverage is n/a.

❗ Current head c24af36 differs from pull request most recent head f316175. Consider uploading reports for the commit f316175 to get more accurate results

@@           Coverage Diff            @@
##           develop    #1023   +/-   ##
========================================
  Coverage    96.03%   96.03%           
========================================
  Files          133      133           
  Lines         3106     3106           
========================================
  Hits          2983     2983           
  Misses         123      123           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Sep 14, 2022

File comparision analysis report

🔖 Info

Target Flash Used (%) Flash Available (%) Static RAM (%)
bootloader 181884 (69%) 80260 (30%) 37456 (7%)
os 412860 (26%) 1151812 (73%) 81376 (15%)
Click to show memory sections
| -          |      Hex |     Bytes |  KiB |
|------------|---------:|----------:|-----:|
| Flash      | 0x200000 | 2 097 152 | 2048 |
| SRAM       |  0x80000 |   524 288 |  512 |
| Bootloader |  0x40000 |   262 144 |  256 |
| Header     |   0x1000 |     4 096 |    4 |
| OS         | 0x17E000 | 1 564 672 | 1528 |
| Tail       |   0x1000 |     4 096 |    4 |
| Scratch    |  0x40000 |   262 144 |  256 |

📝 Summary

Click to show summary
  • ✔️ - existing target
  • ✨ - new target
  • ⚰️ - deleted target
  • ✅ - files are the same
  • ❌ - files are different
Target Status .bin .map Total Flash (base/head) Total Flash Δ Static RAM (base/head) Static RAM Δ
LekaOS ✔️ 412860 (19%) ø 81376 (15%) ø
bootloader ✔️ 181884 (8%) ø 37456 (7%) ø
spike_lk_audio ✔️ 132380 (6%) ø 20984 (4%) ø
spike_lk_behavior_kit ✔️ 195476 (9%) ø 54528 (10%) ø
spike_lk_ble ✔️ 233676 (11%) ø 34712 (6%) ø
spike_lk_bluetooth ✔️ 92236 (4%) ø 18064 (3%) ø
spike_lk_cg_animations ✔️ 152264 (7%) ø 53064 (10%) ø
spike_lk_color_kit ✔️ 88352 (4%) ø 20216 (3%) ø
spike_lk_command_kit ✔️ 200020 (9%) ø 57864 (11%) ø
spike_lk_config_kit ✔️ 136100 (6%) ø 20976 (4%) ø
spike_lk_coreled ✔️ 87916 (4%) ø 20104 (3%) ø
spike_lk_event_queue ✔️ 84088 (4%) ø 18744 (3%) ø
spike_lk_file_manager_kit ✔️ 143648 (6%) ø 21304 (4%) ø
spike_lk_file_reception ✔️ 331036 (15%) ø 34072 (6%) ø
spike_lk_flash_memory ✔️ 86712 (4%) ø 18056 (3%) ø
spike_lk_fs ✔️ 171216 (8%) ø 47968 (9%) ø
spike_lk_lcd ✔️ 169100 (8%) ø 53288 (10%) ø
spike_lk_led_kit ✔️ 115708 (5%) ø 21024 (4%) ø
spike_lk_log_kit ✔️ 84440 (4%) ø 19248 (3%) ø
spike_lk_motors ✔️ 86008 (4%) ø 18088 (3%) ø
spike_lk_qdac ✔️ 91508 (4%) ø 18616 (3%) ø
spike_lk_reinforcer ✔️ 112188 (5%) ø 21024 (4%) ø
spike_lk_rfid ✔️ 84636 (4%) ø 18024 (3%) ø
spike_lk_sensors_battery ✔️ 87056 (4%) ø 19120 (3%) ø
spike_lk_sensors_light ✔️ 83944 (4%) ø 18056 (3%) ø
spike_lk_sensors_microphone ✔️ 84696 (4%) ø 18056 (3%) ø
spike_lk_sensors_temperature_humidity ✔️ 90336 (4%) ø 18032 (3%) ø
spike_lk_sensors_touch ✔️ 91608 (4%) ø 18296 (3%) ø
spike_lk_serial_number ✔️ 125820 (5%) ø 20896 (3%) ø
spike_lk_ticker_timeout ✔️ 82584 (3%) ø 18072 (3%) ø
spike_lk_update_process_app_base ✔️ 145284 (6%) ø 21904 (4%) ø
spike_lk_update_process_app_update ✔️ 100328 (4%) ø 19080 (3%) ø
spike_lk_watchdog_isr ✔️ 87940 (4%) ø 19960 (3%) ø
spike_lk_wifi ✔️ 130640 (6%) ø 21368 (4%) ø
spike_mbed_blinky ✔️ 57616 (2%) ø 11496 (2%) ø
spike_mbed_watchdog_ticker_vs_thread ✔️ 84112 (4%) ø 18920 (3%) ø
spike_stl_cxxsupport ✔️ 83424 (3%) ø 18144 (3%) ø

🗺️ Map files diff output

Click to show diff list

No differenes where found in map files.

@YannLocatelli YannLocatelli force-pushed the yann/tools/set-firmware-version-from-makefile branch from fadf1dc to 50e14bf Compare September 14, 2022 14:51
@github-actions
Copy link

github-actions bot commented Sep 14, 2022

File comparision analysis report

🔖 Info

Target Flash Used (%) Flash Available (%) Static RAM (%)
bootloader 169508 (64%) 92636 (35%) 30920 (5%)
os 374768 (23%) 1189904 (76%) 74448 (14%)
Click to show memory sections
| -          |      Hex |     Bytes |  KiB |
|------------|---------:|----------:|-----:|
| Flash      | 0x200000 | 2 097 152 | 2048 |
| SRAM       |  0x80000 |   524 288 |  512 |
| Bootloader |  0x40000 |   262 144 |  256 |
| Header     |   0x1000 |     4 096 |    4 |
| OS         | 0x17E000 | 1 564 672 | 1528 |
| Tail       |   0x1000 |     4 096 |    4 |
| Scratch    |  0x40000 |   262 144 |  256 |

📝 Summary

Click to show summary
  • ✔️ - existing target
  • ✨ - new target
  • ⚰️ - deleted target
  • ✅ - files are the same
  • ❌ - files are different
Target Status .bin .map Total Flash (base/head) Total Flash Δ Static RAM (base/head) Static RAM Δ
LekaOS ✔️ 374768 (17%) ø 74448 (14%) ø
bootloader ✔️ 169508 (8%) ø 30920 (5%) ø
spike_lk_audio ✔️ 122324 (5%) ø 14568 (2%) ø
spike_lk_behavior_kit ✔️ 186992 (8%) ø 48112 (9%) ø
spike_lk_ble ✔️ 225792 (10%) ø 28056 (5%) ø
spike_lk_bluetooth ✔️ 82948 (3%) ø 11544 (2%) ø
spike_lk_cg_animations ✔️ 144632 (6%) ø 46528 (8%) ø
spike_lk_color_kit ✔️ 65712 (3%) ø 13744 (2%) ø
spike_lk_command_kit ✔️ 189760 (9%) ø 50936 (9%) ø
spike_lk_config_kit ✔️ 124188 (5%) ø 14312 (2%) ø
spike_lk_coreled ✔️ 76164 (3%) ø 13688 (2%) ø
spike_lk_event_queue ✔️ 74736 (3%) ø 12072 (2%) ø
spike_lk_file_manager_kit ✔️ 128680 (6%) ø 14384 (2%) ø
spike_lk_file_reception ✔️ 326968 (15%) ø 27576 (5%) ø
spike_lk_flash_memory ✔️ 63880 (3%) ø 11448 (2%) ø
spike_lk_fs ✔️ 171856 (8%) ø 47880 (9%) ø
spike_lk_lcd ✔️ 159468 (7%) ø 46616 (8%) ø
spike_lk_led_kit ✔️ 103992 (4%) ø 14608 (2%) ø
spike_lk_log_kit ✔️ 63736 (3%) ø 12256 (2%) ø
spike_lk_motors ✔️ 62600 (2%) ø 11488 (2%) ø
spike_lk_qdac ✔️ 78564 (3%) ø 11816 (2%) ø
spike_lk_reinforcer ✔️ 103480 (4%) ø 14608 (2%) ø
spike_lk_rfid ✔️ 80920 (3%) ø 11520 (2%) ø
spike_lk_sensors_battery ✔️ 78196 (3%) ø 12568 (2%) ø
spike_lk_sensors_light ✔️ 60056 (2%) ø 11440 (2%) ø
spike_lk_sensors_microphone ✔️ 72496 (3%) ø 11504 (2%) ø
spike_lk_sensors_temperature_humidity ✔️ 67048 (3%) ø 11424 (2%) ø
spike_lk_sensors_touch ✔️ 68600 (3%) ø 11432 (2%) ø
spike_lk_serial_number ✔️ 103876 (4%) ø 14056 (2%) ø
spike_lk_ticker_timeout ✔️ 69052 (3%) ø 11632 (2%) ø
spike_lk_update_process_app_base ✔️ 123476 (5%) ø 15296 (2%) ø
spike_lk_update_process_app_update ✔️ 77632 (3%) ø 12352 (2%) ø
spike_lk_watchdog_isr ✔️ 82032 (3%) ø 13288 (2%) ø
spike_lk_wifi ✔️ 116392 (5%) ø 14808 (2%) ø
spike_mbed_blinky ✔️ 57968 (2%) ø 11400 (2%) ø
spike_mbed_watchdog_ticker_vs_thread ✔️ 63208 (3%) ø 12448 (2%) ø
spike_stl_cxxsupport ✔️ 58456 (2%) ø 11400 (2%) ø

🗺️ Map files diff output

Click to show diff list

No differenes where found in map files.

Copy link
Member

@ladislas ladislas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, LGTM 👍

I like the fact that we are using make firmware but the version itself should be defined in a config file, not in the Makefile

tools/firmware/build_firmware.sh Outdated Show resolved Hide resolved
tools/firmware/build_firmware.sh Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@YannLocatelli YannLocatelli force-pushed the yann/tools/set-firmware-version-from-makefile branch from 082c5a3 to 5872cec Compare September 16, 2022 13:22
@lgtm-com
Copy link

lgtm-com bot commented Sep 16, 2022

This pull request introduces 2 alerts when merging 5872cec into acc530d - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'

@YannLocatelli
Copy link
Member Author

Suggest that build come in another PR

Makefile Outdated Show resolved Hide resolved
@YannLocatelli YannLocatelli force-pushed the yann/tools/set-firmware-version-from-makefile branch from 5872cec to 78c784d Compare September 16, 2022 17:30
@lgtm-com
Copy link

lgtm-com bot commented Sep 16, 2022

This pull request introduces 2 alerts when merging 78c784d into acc530d - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'

Copy link
Member

@ladislas ladislas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes, this is much better 👍

I find the python script a bit over engineered/complicated to be honest.

I'm not sure we need to pass arguments, especially the standards, and even the input could be hardcoded (see my comment about using a path instead of the string for CI)

The checks are quite simple, super fast, so scripting/functional programming should be preferred to OOP.

Let's discuss that on Monday :)

Makefile Outdated Show resolved Hide resolved
tools/check_version.py Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
config/leka_os_version Outdated Show resolved Hide resolved
tools/check_version.py Outdated Show resolved Hide resolved
tools/check_version.py Outdated Show resolved Hide resolved
tools/check_version.py Outdated Show resolved Hide resolved
tools/check_version.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Sep 20, 2022

This pull request introduces 3 alerts when merging c24af36 into fb30049 - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'
  • 1 for Use of the return value of a procedure

tools/check_version.py Outdated Show resolved Hide resolved
@YannLocatelli YannLocatelli force-pushed the yann/tools/set-firmware-version-from-makefile branch from c24af36 to 8a9b906 Compare September 20, 2022 16:32
@lgtm-com
Copy link

lgtm-com bot commented Sep 20, 2022

This pull request introduces 2 alerts when merging 8a9b906 into fb30049 - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'

@YannLocatelli YannLocatelli force-pushed the yann/tools/set-firmware-version-from-makefile branch from 8a9b906 to d5861e8 Compare September 20, 2022 17:12
@lgtm-com
Copy link

lgtm-com bot commented Sep 20, 2022

This pull request introduces 2 alerts when merging d5861e8 into 3e90d9b - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'

@YannLocatelli
Copy link
Member Author

YannLocatelli commented Sep 20, 2022

  • Validation on application-signed file + robot with github CI on 20/09/2022

…file

- Add regex to check format of version
- Add script to check version is compliant with semver and mcuboot
@ladislas ladislas force-pushed the yann/tools/set-firmware-version-from-makefile branch from d5861e8 to f316175 Compare September 22, 2022 09:00
@ladislas ladislas merged commit da720a0 into develop Sep 22, 2022
@lgtm-com
Copy link

lgtm-com bot commented Sep 22, 2022

This pull request introduces 2 alerts when merging f316175 into 3e90d9b - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'

@ladislas ladislas deleted the yann/tools/set-firmware-version-from-makefile branch September 22, 2022 09:05
@sonarcloud
Copy link

sonarcloud bot commented Sep 22, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
01 - type: task Something to do
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants