Skip to content

Update exporters to include and use mbed_conf.h #1964

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

Closed
wants to merge 4 commits into from

Conversation

theotherjimmy
Copy link
Contributor

@theotherjimmy theotherjimmy commented Jun 16, 2016

Affected exporters:

  • gcc_arm
  • uVision4
  • IAR
  • emblocks
  • atmelstudio
  • codered
  • simplicityv3

@theotherjimmy
Copy link
Contributor Author

cc @bogdanm @0xc0170

@theotherjimmy theotherjimmy force-pushed the export-mbed-conf branch 3 times, most recently from a808b89 to ec98a3b Compare June 16, 2016 18:40
def get_compile_options(self, defines, includes):
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]
opts = opts + self.get_conifg_option(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.

Shouldn't this be self.get_config_option()? :) This is the problem the CI is complaining about.

@theotherjimmy
Copy link
Contributor Author

thanks @screamerbg !

@screamerbg
Copy link
Contributor

@mbed-bot: TEST

HOST_OSES=windows
BUILD_TOOLCHAINS=GCC_ARM,ARM,IAR
TARGETS=K64F,KL25Z

@mbed-bot
Copy link

Merged build finished. No test results found.
[Build 505]
SUCCESS: Building succeeded and tests were run! Be sure to check the test results

@mbed-bot
Copy link

[Build 505]
SUCCESS: Building succeeded and tests were run! Be sure to check the test results

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 17, 2016

There's one PR to change the name of the config (#1968), we should wait to see what's the final name of the config header file.

I'll fetch this and test locally. The CI tests results are good.


# Add the configuration file to the target directory
self.config_header = "mbed_conf.h"
cfg.get_config_data_header(join(trg_path, self.config_header))
Copy link
Contributor

@0xc0170 0xc0170 Jun 17, 2016

Choose a reason for hiding this comment

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

cfg is not defined in this context NameError: global name 'cfg' is not defined. should be config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right. the rebase renamed it to config.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 17, 2016

The uvision requires a fix, it does not use progen_flags() function, like the rest of exporters do, thus it does not get mbed config into c_flags.

I exported to IAR, c_flags look good, not certain if preinclude should be together with mbed_conf, as this puts them on the lines in the misc options in the IDE : --preincludembed_conf.h

{'c_flags': ['--preinclude', 'mbed_conf.h'], 'asm_flags': [], 'common_flags': ['--no_wrap_diagnostics', '-e', '--diag_suppress=Pa050,Pa084,P
a093,Pa082'], 'ld_flags': ['--skip_dynamic_initialization', '--threaded_lib'], 'cxx_flags': ['--guard_calls', '--preinclude', 'mbed_conf.h']
}                                                                                                                                           

Plus now that we have this header file, Warning[Pe047]: incompatible redefinition of macro "UNITY_INCLUDE_CONFIG_H" C:\2\mbed-blinky-morpheus\projectfiles\iar\mbed_conf.h 10

-D is still present

@bogdanm
Copy link
Contributor

bogdanm commented Jun 17, 2016

-D is still present

I think we need a new attribute for exporters, something like support_config_header, since we're not going to convert all of them to use prefix headers at once. This will help in eliminating the duplication.

@theotherjimmy
Copy link
Contributor Author

I can't push to this branch anymore. It's claiming that I don't have permission.

@theotherjimmy
Copy link
Contributor Author

since we're not going to convert all of them to use prefix headers at once.

We could.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 18, 2016

@theotherjimmy Please always use the forked branches. Anyway, seems like there was a change within the permission, I don't know :( The fastest way would be to use your own fork and replace this PR with a new one.

@theotherjimmy
Copy link
Contributor Author

discussion should follow on #1975

bogdanm added a commit that referenced this pull request Jun 28, 2016
[Exporters] Update exporters to include and use mbed_conf.h (Was #1964)
@screamerbg screamerbg deleted the export-mbed-conf branch July 13, 2016 18:48
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.

5 participants