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

e2 studio exporter update #4959

Merged
merged 1 commit into from
Sep 25, 2017
Merged

e2 studio exporter update #4959

merged 1 commit into from
Sep 25, 2017

Conversation

phyokyaw
Copy link

@phyokyaw phyokyaw commented Aug 23, 2017

#4942

Description

This change removes legacy e2 studio IDE exporter and updated to inherit from gnuarmeclipse exporter. Only RZ-A1H .cproject template is updated.

Status

READY

Migrations

NO

Deploy notes

The exported project wont build unless Cortex-A support is added to OS 5.5.
https://developer.mbed.org/teams/Renesas/wiki/How-to-checkout-older-mbed-OS-5-branch

This is related to #4624.

Copy link
Contributor

@theotherjimmy theotherjimmy left a comment

Choose a reason for hiding this comment

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

This is headed in the right direction. For the best user experience, an exporter:

  • Takes input from the resource scan.
  • Uses the flags in the build profiles.
  • Has a single template file for each file type they produce. For example, an eclipse CDT project would have one template for .project files and one for .cproject files.
  • Does not call mbed CLI. It is possible to export from the website, which will not include mbed CLI in the resulting zip.

print 'Project name: {0}'.format(self.project_name)
print 'Target: {0}'.format(self.toolchain.target.name)
print 'Toolchain: {0}'.format(self.TOOLCHAIN)
def collect_tmpl_vars(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rename this "create_jinja_ctx"? That's what it does after all.

self.gen_file('gnuarmeclipse/makefile.targets.tmpl', jinja_ctx, 'makefile.targets', trim_blocks=True, lstrip_blocks=True)

print
print 'Done. Import the \'{0}\' project in Eclipse.'.format(self.project_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change this to say "Import the project located at \'{0}\'.format(self.export_dir)? That way the user knows exactly what to do.

@theotherjimmy
Copy link
Contributor

Most of the way there. I would like to see a single template file before this is merged though.

@phyokyaw
Copy link
Author

@mbedNoobNinja, Hello :-) I am in the process of updating e2 studio exporter for RZ-A1H and I believe you added support for VK-RZA1H as part of #3632. As RZ-A1H and VK-RZA1H require same template file I'll remove VK-RZA1H templates you added to single template.

@phyokyaw
Copy link
Author

@theotherjimmy Hi Jimmy, pushed amended patch based on your feedback. Please review if you have time. Thanks.

@theotherjimmy
Copy link
Contributor

Looks great! I'm glad you were able to remove the duplication in this exporter. It looks like your .cproject project file template might be the same as the gnuarmeclipse exporter's template. Is that the case? If so, could you just use the gnuarmeclipse one?

@phyokyaw
Copy link
Author

Thanks for the reply. e2 studio extends gnuarmeclipse toolchain IDs in .cproject file. So we can't really reuse .cproject file from gnuarmeclipse exporter directly. There are minor differences on IDs in a few places.

@theotherjimmy
Copy link
Contributor

Thanks for the quick response. What do we need to add to the CI machines to make it possible to regression test this exporter as part of morph export-build?

@phyokyaw
Copy link
Author

Just wondering how CI regression test is done for gnuarmeclipse exporter? We can do similar. It wont be simple to test if the exported project builds on e2 studio build.

@JojoS62
Copy link
Contributor

JojoS62 commented Aug 30, 2017

I'm working on the MCUXpresso exporter, it uses also the gnuarmeclipse as a base. There is a build method that calls the IDE in 'headless' mode, which means the build process is started without gui. For MCUX, I had to change only the cmd name because its different than 'eclipse'. For e2 it may be also the only modification.
For testing I have started the process in the cmd window with the arguments in the build method.
The mbed team need to install the e2studio software on their testing machine and add this exporter to the tests.

@phyokyaw
Copy link
Author

Many thanks for response and info.

There is a build method that calls the IDE in 'headless' mode, which means the build process is started without gui.

I assume CI uses
https://gnu-mcu-eclipse.github.io/advanced/headless-builds/
e2 studio command is still 'eclipse', so no need to change and should build as standard eclipse. e2 studio is downloadable and should be able to add to tests as standard eclipse.

@theotherjimmy The exported project on CI won't be built because latest CMSIS-RTOS mbed OS 5.5 doesn't support RZ-A1H (Cortex-A). Please let me know the status on this.

Thanks

@theotherjimmy
Copy link
Contributor

@phyokyaw You make a very good point. However, I would still like to make sure that if we enable support in the future, after turning on Cortex-A support, that it will "just work" in the sense that it will test this exporter in CI after that point forward.

@phyokyaw
Copy link
Author

phyokyaw commented Sep 4, 2017

Is there anything I need to do from my side for this work?

@theotherjimmy
Copy link
Contributor

@phyokyaw Nope. We have a bit of a backup in CI tasks. We're probably done with the yak shaving, and we might be able to add this exporter to CI soon.

@theotherjimmy
Copy link
Contributor

/morph export-build

@mbed-bot
Copy link

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph export-build

Output

mbed Build Number: 150

All exports and builds passed!

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.

6 participants