Skip to content

Configuration in header files - step 1 #1957

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

Merged
merged 4 commits into from
Jun 16, 2016
Merged

Configuration in header files - step 1 #1957

merged 4 commits into from
Jun 16, 2016

Conversation

bogdanm
Copy link
Contributor

@bogdanm bogdanm commented Jun 15, 2016

The current implementation of the configuration system "compiles" the
configuration parameters to macros defined on the command line. This
works, but has a few limitations:

  • it can bring back the maximum command line length issues in Windows.
  • depending on the type of the configuration parameter, it might require
    special quoting of its value in the command line.
  • various 3rd party IDE/tools seem to have some limitations regarding
    the total length of the macros that can be defined.

This PR is the first step in replacing the current mechanism with
one that generates configuration in header files that will be
automatically included, instead of command line macro definitions.After
this PR is accepted, compiling from the command line will use these
generated header files. Exporters still use the previous mechanism
(a list of macros for the configuration parameters).

The commit also fixes an issue in tools/get_config.py that appeared as a
result of a recent PR: the signature of "get_config" in
tools/build_api.py changed, but tools/get_config.py was not updated.

The current implementation of the configuration system "compiles" the
configuration parameters to macros defined on the command line. This
works, but has a few limitations:

- it can bring back the maximum command line length issues in Windows.
- depending on the type of the configuration parameter, it might require
  special quoting of its value in the command line.
- various 3rd party IDE/tools seem to have some limitations regarding
  the total length of the macros that can be defined.

This commit is the first step in replacing the current mechanism with
one that generates configuration in header files that will be
automatically included, instead of command line macro definitions. The
commit only adds the method for generating the header file, it doesn't
actually change the way config is used yet (that will happen in step 2),
thus it is backwards compatible. The logic of the configuration system
itself is unchanged (in fact, the whole change (not only this commit) is
supposed to be completely transparent for the users of the configuration
system).

The commit also fixes an issue in tools/get_config.py that appeared as a
result of a recent PR: the signature of "get_config" in
tools/build_api.py changed, but tools/get_config.py was not updated.
@bogdanm
Copy link
Contributor Author

bogdanm commented Jun 15, 2016

@0xc0170 @geky

header_data += "// Configuration parameters\n"
for m in params.values():
if m.value is not None:
header_data += "#define %s%s%s%s// set by %s\n" % (m.macro_name, " " * (max_macro_name_len - len(m.macro_name) + 1), str(m.value),
Copy link
Contributor

Choose a reason for hiding this comment

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

this could also be

header_data += "#define {0:<{1}} {2:<{3}} // set by {4}".format(m.macro_name, max_macro_name_len, str(m.value), max_macro_val_len, m.set_by)

that might be a bit more clear? or at least, it gets rid of the math.

@bogdanm
Copy link
Contributor Author

bogdanm commented Jun 16, 2016

Thanks @theotherjimmy, string formatting looks much better now.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 16, 2016

Thanks for this support !

LGTM, but without the step 2 we can't test it (or at least we can write a python test to generate the header config file and read it back for verification)

@bogdanm
Copy link
Contributor Author

bogdanm commented Jun 16, 2016

LGTM, but without the step 2 we can't test it

Definitely. I "verified" it visually by dumping all the test data to stdout in header format, but of course that's not enough. In step 2 I'll verify using some applications that use the configuration system. If you'd rather have the toolchain modifications in this PR instead of a new one, I can do that too.

@bogdanm
Copy link
Contributor Author

bogdanm commented Jun 16, 2016

OK, I'll add the toolchain changes then. Please do not merge this PR yet.

This commit uses the previously introduced feature of generating
configuration data as a C header file rather than as command line macro
definitions. Each toolchain was modified to use prefix headers if
requested, and build_api.py was modified to set up the toolchain's
prefix header content using the data generated by the config system.

Tested by compiling blinky for GCC and ARMCC. I'm having a few issues
with my IAR license currently, but both ARMCC and IAR use the same
`--preinclude` option for prefix headers, so this shouldn't be an issue.

Note that at the moment all exporters still use the previous
configuration data mechanism (individual macro definitions as opposed to
a prefix header). Exporters will be updated in one or more PRs that will
follow.
@bogdanm
Copy link
Contributor Author

bogdanm commented Jun 16, 2016

OK, toolchain support is now in and the description of the PR was modified to reflect this change. Let's see how the CI feels about it.

def get_prefix_header(self):
if self.prefix_header_content is None:
return None
prefix_file = join(self.build_dir, "mbed_prefix.h")
Copy link
Contributor

Choose a reason for hiding this comment

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

mbed_config.h, wouldn't that be better name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as the toolchain is concerned, this is a prefix header, it doesn't know anything about the config system (which is completely external). However, that can change. I can modify some function names to make it clear that this feature is to be used by the config system, then modify the header name to mbed_conf.h (not mbed_config.h, since I believe that's a different mechanism that we introduced a while ago).

Copy link
Contributor

Choose a reason for hiding this comment

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

mbed_conf.h +1

@bogdanm
Copy link
Contributor Author

bogdanm commented Jun 16, 2016

OK, fixed names ... For reference, this is how the config header looks like:

 $ cat .build/K64F/GCC_ARM/mbed_config.h 
// Automatically generated configuration file.
// DO NOT EDIT, content will be overwritten.

#ifndef __MBED_CONFIG_DATA__
#define __MBED_CONFIG_DATA__

// Configuration parameters
#define MBED_CONF_RTOS_PRESENT 1 // set by library:rtos
// Macros
#define UNITY_INCLUDE_CONFIG_H   // defined by library:utest

#endif

@bogdanm
Copy link
Contributor Author

bogdanm commented Jun 16, 2016

Ah damn it. Sorry, mbed_conf.h. Fix coming.

Also changed some function names to make it clear that the prefix
headers feature is only used for config.
@bogdanm
Copy link
Contributor Author

bogdanm commented Jun 16, 2016

OK, it's mbed_conf.h now (forced push).

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 16, 2016

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=K64F,LPC1768,NUCLEO_F411RE

opts = ['-D%s' % d for d in defines] + ['@%s' % self.get_inc_file(includes)]
config_header = self.get_config_header()
if config_header is not None:
opts = opts + ['-include', config_header]
Copy link
Contributor

Choose a reason for hiding this comment

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

-imacros might be better?
from the gnu gcc manual:

Exactly like -include, except that any output produced by scanning file is thrown away. Macros it defines remain defined. This allows you to acquire all the macros from a header without also processing its declarations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see how that changes anything?

Copy link
Contributor

Choose a reason for hiding this comment

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

the -include option does:

Process file as if #include "file" appeared as the first line of the primary source file. However, the first directory searched for file is the preprocessor's working directory instead of the directory containing the main source file. If not found there, it is searched for in the remainder of the #include "..." search chain as normal.

Copy link
Contributor

@theotherjimmy theotherjimmy Jun 16, 2016

Choose a reason for hiding this comment

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

The difference is that declarations in a -included file are heeded where as in a -imacrosed file they are not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant I don't see how that changes the current functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

It enforces the restriction that the configuration be only preprocessor defines.

@mbed-bot
Copy link

[Build 500]
FAILURE: Something went wrong when building and testing.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 16, 2016

The test results are good !

LGTM

@theotherjimmy
Copy link
Contributor

I'm not seeing the "mbed_conf.h" file
I do find -name "mbed_conf.h" after a build and get no results.

@theotherjimmy
Copy link
Contributor

Found it. looks good 👍

@0xc0170 0xc0170 merged commit 1e18ea2 into master Jun 16, 2016
@sg- sg- deleted the config_headers branch October 18, 2016 13:48
artokin pushed a commit to artokin/mbed-os that referenced this pull request Feb 25, 2019
…..c5ee9e4

c5ee9e4 Remove content from unit tests
f7ca82a Merge branch 'release_internal' into release_external
b400a6a Fix for pr ARMmbed#1984 (ARMmbed#1987)
30d25bc Fix compiler warnings (ARMmbed#1986)
d46d7b3 Prioritise thread control messages  (ARMmbed#1984)
e59dbd8 Update domain address lifetime (ARMmbed#1985)
8a5cb75 Merge pull request ARMmbed#1980 from ARMmbed/sync_with_MbedOS
bd6feab Update coding style
ff14e80 (via Mbed OS) nanostack: icmpv6: fix build warning
f5e3423 (via Mbed OS) Require dependencies from nanostack mbed_lib.json
3aec837 Restore ws_management_api.h to the latest version
cd7ae3f (via Mbed OS) Review changes corrected
4f23008 (via Mbed OS) This is a initial version of Wi-Sun interface implementation. To get Wi-Sun mesh network working, also nanostack with Wi-Sun support is needed. ws_empty_functions.c and ws_management_api.h are temporary included here, so that wisun_tasklet will compiled without problems. They will replaced with the official versions with next nanostack release.
e029444 Merge pull request ARMmbed#1981 from ARMmbed/iotthd-3111
d7e8aea Path control size from 5->7.
a5fe76f randomise challenge tlv for parent request retries. (ARMmbed#1977)
a1c8277 Merge pull request ARMmbed#1978 from ARMmbed/IOTTHD-3219
62f8b41 Fix compiler warnings
1ec7a84 Fix issues found by coverity
5fe7120 Merge pull request ARMmbed#1974 from ARMmbed/iotthd-3100
51358f9 Removed Unncessary debug print.
136d1b1 Merge pull request ARMmbed#1973 from ARMmbed/IOTTHD-3018
4e557ae DHCPv6 update:
85b33e1 Update BBR prefix assignment (ARMmbed#1972)
899b2c5 DHCPv6 client and server update:
9009eaf Update copyright year to test files
6a56318 Update copyright year to Nanostack files
a83472e Added Wi-Sun certificate and security test interfaces
7f4ebf1 Corrected compiler warnings
52e4c3f Added supplicant PAE NVM storage
92df57b Added key data access functions to key storage
5972bc3 Merge pull request ARMmbed#1959 from ARMmbed/ws_eapol_ie_update
98af118 Merge pull request ARMmbed#1965 from ARMmbed/IOTTHD-3193
fc76d1e Merge pull request ARMmbed#1963 from ARMmbed/rename_socket_h
ba3a649 Added WS flagging to EAP header parser
048f14a Rename address.h
5739b4a Fixed aro registration error
887d931 Rename socket.h to ns_socket.h
7ebaa8e Add Nanostack configuration for WS (ARMmbed#1961)
ed87161 Code style fix.
e20028a Fix compiler warnings (ARMmbed#1957)
b86f885 EAPOL data flow IE update
65472de Fixed Function protype typo.
f539287 Added support for write/READ  EA-IE header IE Element's
ce72b55 Merge pull request ARMmbed#1958 from ARMmbed/tls_conf_err
1e8b18c Added handling for mbed TLS configuration error
bdfea40 WS: Use common channel number calc function
4802aae Merge pull request ARMmbed#1930 from ARMmbed/IOTTHD-3027
2b8c846 PAE BR address write/read interface
9777ad1 Remove yotta references (ARMmbed#1954)
af8890b Merge pull request ARMmbed#1949 from ARMmbed/eapol_eap_and_tls
c546d4f WS: Fixed EU domain channel numbers in neighbor class
cff6f0b WS: Missing return value fix
ebcdba5 Changed EAP-TLS identity to anonymous
34d2f15 Corrected defects and coding style
79c7157 WS: Default domain config update
0724863 Merge branch 'master' into IOTTHD-3027
65ccc41 WS: RF config set in own functions
76d235e MAC: Fixed virtual driver warnings
88641c1 Updates to PAEs and other security protocols
f57138f Security key storage and certificate info updates
b7177c5 TLS security protocol and mbed TLS security protocol library
9c9e3c9 EAP-TLS protocol implementation
9c5fc92 WS: Fixed code style
acce0dd Added empty function for test
4b28192 Added support for ARO registration failure
9d251fa Added empty function fr new interface
61f520f Added api to configure network size parameters
4d26258 Flagged mbed TLS KW header and corrected bool definitions
3d903fa Corrected NIST AES KW flagging
2f4e099 Removed temporary KW functions and corrected ut and style
6481549 Added GKH MIC validation and encryption
cc3ce58 Moved 4WH functions to library and added constants
650771c Added unit test to NIST AES KW library
6a82e7d added parent priority handling. (ARMmbed#1942)
7b7f1c1 Merge pull request ARMmbed#1945 from ARMmbed/fix_synch_parent_warn
38b28e2 Fix coverity error (ARMmbed#1943)
c4afedc thread_mle_message_handler: fix build warning (ARMmbed#1940)
a5be64a Follow Mbed OS coding style (ARMmbed#1941)
6298cef Sync mbed_lib.json with Mbed OS (ARMmbed#1935)
afe3ec6 Added Wi-Sun flagging to eapol helper
28d10d6 Merge pull request ARMmbed#1901 from ARMmbed/kmp_pae_init
3f56121 Disabled EAPOL flags
97f07a9 Tuned EAPOL timers for small networks
6e063b6 added Wi-SUN neighbor table management to Wi-SUN
7f4c61d Corrected KMP api start on authenticator
91ca2e6 Corrected KMP timer active check and security protocol address get
65d983f If mbedtls NIST AES KW is not enabled defined it as null algorithm
16dbe27 Added unit test stub for PAE controller
a524936 Moved EAPOL relay port and IP address configuration to bootstrap
68fa4f6 Added extra debug info flags to new security libraries
c4411ea Removed extra memory frees from KMP socket and eapol if and fixed KMP comments
8088a86 Added and fixed security protocols comments
cc32457 Modified PAEs to use protocol core timer function call
03469f3 Modified PAE entities to be bound to interface
369c8a0 Added 4WH integrity protection and encryption
ebae1d5 Added HMAC-SHA1, IEEE 802.11 PRF and NIST AES KW libraries
2d50887 Corrected Wi-SUN security component initializations
8a8b6ef Added configuration flags for supplicant and authenticator PAEs and EAPOL relay
3868ff1 PAE and security protocols timer support
c61066a Corrected GKH to set eapol-KEY message group key negotation bit correctly
dfe52d9 Added 4WH,GKH and EAP-TLS module and modified kmp service
782f3fb Relay message flow optimization
bbd6ee1 Integrate EAPOL new  encode and decode functionality
1802ee7 EAPOL message parser and write helper function.
da15653 EAPOL relay and KMP changes
fad633f EAPOL relay
cc97054 Unicast Shedule update
a099524 EAPOL authtentication start fix
3f32a7a Fix valgrind uninitilaized data use.
6e9f6a4 Corrected memory errors and some compiler warnings
c5f1af3 Initial EAPOL changes
e48aa79 WS: Use Tack in Ack wait time
59a65ea Change FHSS timing defaults
be06ecb FHSS WS: Fixed synch parent warning
a3aa38b Thread extension commission updates (ARMmbed#1870)
3e89d0a Multicast registrations update (ARMmbed#1931)
e18055a MAC: Update symbol rate when RF configuration changed
9eada28 WS: Store RF configuration
35145a3 MAC: RF configuration delivery implemented
d6b2bbc Merge pull request ARMmbed#1928 from ARMmbed/IOTTHD-3028
80683e2 MAC/WS: Implemented Ack wait duration set
0e9ead2 Remove excess tracing (ARMmbed#1927)
ae210cd Merge pull request ARMmbed#1923 from ARMmbed/IOTTHD-3080
17fad47 Merge pull request ARMmbed#1924 from ARMmbed/IOTTHD-1608
6fd6bd5 Update thread neighbor table initialisation (ARMmbed#1926)
c09d38a Merge pull request ARMmbed#1925 from ARMmbed/fix_protocol_if_fhss
9ac5301 MAC MLME: Removed BEA TX trace which was causing stack overflow
5dee7b1 FHSS: Use debug callback in TX/RX slot switch
f779fad WS/Protocol: Fixed getting interface pointer using FHSS api
4177fa4 Prevalidate CoAP msg source address
5451b1a FHSS unit tests: Updated timestamp callback
a2997b1 FHSS: Removed debug trace which was causing crash
3b6a921 FHSS: Debug callback update
ec1c41d FHSS: Use given TX time in synch calculation
f0c0f66 MAC: Write FHSS synch info with tx time
78ea0eb FHSS: Debug callbacks added
c4ef759 MAC: Write FHSS synch info before calling PHY TX function
9cca341 FHSS: Use timestamp delivered by MAC
be46564 Add CoAP message validation
883eb46 Add callback for CoAP message prevalidation (ARMmbed#1918)
59bbe31 Merge pull request ARMmbed#1917 from ARMmbed/IOTTHD-3029
ac4a76e Fixed promiscuous/sniffer mode
bc2fb64 FHSS WS: time convert to support negative values
7dce509 FHSS WS: Clock drift compensation implemented
ac7c90a Fix Thread resolution client initialisation (ARMmbed#1915)
b744186 Set the default unicast channel function as Direct Hash
612c4b5 Update DHCP to follow Wi-SUN specification
eeb5168 IPv6 route metrics update (ARMmbed#1912)
1debcca Fix compilation warnings noticed in mbed-os (ARMmbed#1909)
0a18231 Change ARO routes to be direct route instead of on-link
7fb321e Merge pull request ARMmbed#1906 from ARMmbed/parent_update_fix
b5afc35 Fixed missing broadcast synch information loose with default zeroes.
3dbc874 Add missing closing bracket
0d746f0 Changed Wi-SUN HW type to match specification
2804bf4 Enabled roaming and routing between multiple Wi-SUN BR

git-subtree-dir: features/nanostack/sal-stack-nanostack
git-subtree-split: c5ee9e4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants