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

Get setting default values from settings.yaml #6595

Conversation

shellixyz
Copy link
Collaborator

I checked the default are the same as before, nothing has changed

@stronnag
Copy link
Collaborator

Arch Linux :-(

$ arm-none-eabi-gcc -dumpversion
10.2.0
$ ruby -v
ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-linux]

/home/jrh/Projects/fc/inav/src/utils/settings.rb:893:in `block in validate_default_values': Member pinio_box1 default value has an invalid type, integer or symbol expected (RuntimeError)
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:665:in `yield'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:665:in `block (3 levels) in foreach_enabled_member'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:663:in `each'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:663:in `block (2 levels) in foreach_enabled_member'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:661:in `each'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:661:in `block in foreach_enabled_member'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:671:in `each'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:671:in `each'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:671:in `foreach_enabled_member'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:873:in `validate_default_values'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:306:in `write_files'
	from /home/jrh/Projects/fc/inav/src/utils/settings.rb:1099:in `<main>'

@stronnag
Copy link
Collaborator

Above error seems target dependent, in this case MATEKF765

@shellixyz shellixyz force-pushed the improvement/get_setting_default_values_from_settings.yaml branch 2 times, most recently from 8870ab8 to 3ae27bf Compare February 10, 2021 19:16
@shellixyz shellixyz force-pushed the improvement/get_setting_default_values_from_settings.yaml branch 2 times, most recently from 931a65a to 5087026 Compare February 11, 2021 13:24
@stronnag
Copy link
Collaborator

Interesting that we now get linker warnings on some (non -Os?) platforms:

[jrh@eeyore]$ ninja MATEKF405
[362/363] Linking C executable bin/MATEKF405.elf
In function 'sbufWriteData',
    inlined from 'mspFcProcessOutCommand' at ../src/main/fc/fc_msp.c:397:9:
../src/main/common/streambuf.c:71:5: warning: 'memcpy' reading 8 bytes from a region of size 1 [-Wstringop-overflow=]
   71 |     memcpy(dst->ptr, data, len);
      |     ^
Memory region         Used Size  Region Size  %age Used
           FLASH:      498068 B       896 KB     54.29%
    FLASH_CONFIG:          0 GB       128 KB      0.00%
             RAM:       88556 B       128 KB     67.56%
             CCM:       16116 B        64 KB     24.59%
     BACKUP_SRAM:          0 GB         4 KB      0.00%
       MEMORY_B1:          0 GB         0 GB
[363/363] cd /home/jrh/Projects/fc/inav/ninja/sr.../Projects/fc/inav/ninja/inav_2.7.0_MATEKF405.hex
[jrh@eeyore]$ ninja MATEKF722
[362/363] Linking C executable bin/MATEKF722.elf
Memory region         Used Size  Region Size  %age Used
        ITCM_RAM:        6112 B        16 KB     37.30%
      ITCM_FLASH:          0 GB        16 KB      0.00%
ITCM_FLASH_CONFIG:          0 GB        16 KB      0.00%
     ITCM_FLASH1:          0 GB       480 KB      0.00%
           FLASH:         836 B        16 KB      5.10%
    FLASH_CONFIG:          0 GB        16 KB      0.00%
          FLASH1:      402580 B       480 KB     81.91%
             TCM:       16120 B        64 KB     24.60%
             RAM:       79400 B       192 KB   

@stronnag
Copy link
Collaborator

More interesting, shortGitRevsion not passed to the FC

$ cliterm 
open /dev/ttyACM0

Entering CLI Mode, type 'exit' to return, or 'help'

# version
# INAV/WINGFC 2.7.0 Feb 14 2021 / 19:18:55 ()
# GCC-10.2.0

@stronnag
Copy link
Collaborator

And if I build the same target with -DCMAKE_BUILD_TYPE=MinSizeRel, then the board appears to lock up when attempting to connect to the CLI.

Seems like we still have some issues here.

@shellixyz
Copy link
Collaborator Author

shellixyz commented Feb 14, 2021

Interesting that we now get linker warnings on some (non -Os?) platforms:

[jrh@eeyore]$ ninja MATEKF405
[362/363] Linking C executable bin/MATEKF405.elf
In function 'sbufWriteData',
    inlined from 'mspFcProcessOutCommand' at ../src/main/fc/fc_msp.c:397:9:
../src/main/common/streambuf.c:71:5: warning: 'memcpy' reading 8 bytes from a region of size 1 [-Wstringop-overflow=]
   71 |     memcpy(dst->ptr, data, len);
      |     ^
Memory region         Used Size  Region Size  %age Used
           FLASH:      498068 B       896 KB     54.29%
    FLASH_CONFIG:          0 GB       128 KB      0.00%
             RAM:       88556 B       128 KB     67.56%
             CCM:       16116 B        64 KB     24.59%
     BACKUP_SRAM:          0 GB         4 KB      0.00%
       MEMORY_B1:          0 GB         0 GB
[363/363] cd /home/jrh/Projects/fc/inav/ninja/sr.../Projects/fc/inav/ninja/inav_2.7.0_MATEKF405.hex
[jrh@eeyore]$ ninja MATEKF722
[362/363] Linking C executable bin/MATEKF722.elf
Memory region         Used Size  Region Size  %age Used
        ITCM_RAM:        6112 B        16 KB     37.30%
      ITCM_FLASH:          0 GB        16 KB      0.00%
ITCM_FLASH_CONFIG:          0 GB        16 KB      0.00%
     ITCM_FLASH1:          0 GB       480 KB      0.00%
           FLASH:         836 B        16 KB      5.10%
    FLASH_CONFIG:          0 GB        16 KB      0.00%
          FLASH1:      402580 B       480 KB     81.91%
             TCM:       16120 B        64 KB     24.60%
             RAM:       79400 B       192 KB   

Strange, I don't see this warning

EDIT: This seems to be related to your shortGitRevsion issue

@stronnag
Copy link
Collaborator

Also seeing strange SETTINGFAIL errors, even after full erase flash and defaults.

Arming disabled flags: COMPASS ACC HWFAIL RX CLI SETTINGFAIL
### ERROR: Invalid setting: sim_transmit_flags
# get sim_transmit_flags
sim_transmit_flags = 2
Allowed range: 0 - 0

@stronnag
Copy link
Collaborator

Strange, I don't see this warning

EDIT: This seems to be related to your shortGitRevsion issue

Indeed, with gcc 9.3 I don't get any build warnings relating to shortGitRevsion, however shortGitRevsion is still blank at runtime.

# version
# INAV/WINGFC 2.7.0 Feb 14 2021 / 20:29:43 ()
# GCC-9.3.1 20200408 (release)

@stronnag
Copy link
Collaborator

(erroneous comment deleted)

@stronnag
Copy link
Collaborator

@shellixyz finally got to the bottom of the shortGitRevsion problem. Some change to the build generator is causing the provision of the git revision to fail:

# This PR
-D__REVISION__=""
# Master
-D__REVISION__="15321e9c"

@stronnag
Copy link
Collaborator

stronnag commented Feb 16, 2021

@shellixyz finally got to the bottom of the shortGitRevsion problem. Some change to the build generator is causing the provision of the git revision to fail:

# This PR
-D__REVISION__=""
# Master
-D__REVISION__="15321e9c"

This appear to be due to the reordering of CMakeList.txt, such that include(main) which sets ${GIT_REV} is now done before the call to include(GetGitRevisionDescription).

Moving the Git stuff before include(main) resolves the issue.

@shellixyz shellixyz force-pushed the improvement/get_setting_default_values_from_settings.yaml branch 2 times, most recently from ba75b97 to b21814c Compare April 6, 2021 21:53
@stronnag
Copy link
Collaborator

stronnag commented Apr 7, 2021

Looking good.

shellixyz and others added 2 commits April 7, 2021 17:11
- Add support for resolving types and values in multiple compilers
- Add support for resolving types and values in clang 10
- Add support for using the host compiler for resolving the settings

This allows us to run the generator for the unit tests, since they now
need the settings_generated.h file to get the default setting values
from it.
@shellixyz shellixyz force-pushed the improvement/get_setting_default_values_from_settings.yaml branch from 392242e to 438dd40 Compare April 7, 2021 15:13
@shellixyz shellixyz merged commit fc0e5e2 into iNavFlight:master Apr 7, 2021
@giacomo892
Copy link
Collaborator

Great work! Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants