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

jlink: custom flashing support #79372

Merged

Conversation

jcpi-q
Copy link
Contributor

@jcpi-q jcpi-q commented Oct 3, 2024

Allows to use a custom flashing script with jlink runners.

Copy link

github-actions bot commented Oct 3, 2024

Hello @jcpi-q, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@pdgendt
Copy link
Collaborator

pdgendt commented Oct 21, 2024

Requires a rebase

@jcpi-q jcpi-q force-pushed the jc.custom_jlink_flash branch from 7d973f1 to e45204b Compare October 24, 2024 05:01
@jcpi-q jcpi-q force-pushed the jc.custom_jlink_flash branch from e45204b to 43ea33d Compare November 8, 2024 08:40
pdgendt
pdgendt previously approved these changes Nov 8, 2024
@kartben
Copy link
Collaborator

kartben commented Nov 25, 2024

@jcpi-q can you please rebase as there is a merge conflict now? Thanks!

Allows to use a custom flashing script with jlink runners.

Signed-off-by: Jean-Christophe Pince <[email protected]>
@jcpi-q jcpi-q force-pushed the jc.custom_jlink_flash branch from 5a79951 to e2a32d8 Compare November 25, 2024 14:11
@henrikbrixandersen henrikbrixandersen merged commit dd5874b into zephyrproject-rtos:main Nov 25, 2024
36 checks passed
Copy link

Hi @jcpi-q!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

@zeonchew
Copy link
Contributor

zeonchew commented Nov 26, 2024

Hi @jcpi-q

Would like to check with you whether it is intended that after refactoring into run_flash_cmd(), that the construction of cmd variable goes into the condition if self.supports_loader and self.loader: instead of staying outside of the if statement as it was?

def run_flash_cmd(self, fname, flash_file, **kwargs):
loader_details = ""
if self.supports_loader and self.loader:
loader_details = "?" + self.loader
cmd = (
[self.commander]
+ (
['-IP', f'{self.dev_id}']
if (is_ip(self.dev_id) or is_tunnel(self.dev_id))
else (['-USB', f'{self.dev_id}'] if self.dev_id else [])
)
+ (['-nogui', '1'] if self.supports_nogui else [])
+ ['-if', self.iface]
+ ['-speed', self.speed]
+ ['-device', self.device + loader_details]
+ ['-CommanderScript', fname]
+ (['-nogui', '1'] if self.supports_nogui else [])
+ self.tool_opt
)
if flash_file:
self.logger.info(f'Flashing file: {flash_file}')
kwargs = {}
if not self.logger.isEnabledFor(logging.DEBUG):
kwargs['stdout'] = subprocess.DEVNULL
self.check_call(cmd, **kwargs)

west flash command stops to works at my side because variable cmd is not initialized, as the loader option is not used.
Hence causing exception to the thrown at the execution line at L438:

self.check_call(cmd, **kwargs)

@kartben
Copy link
Collaborator

kartben commented Nov 26, 2024

Hi @jcpi-q

Would like to check with you whether it is intended that after refactoring into run_flash_cmd(), that the construction of cmd variable goes into the condition if self.supports_loader and self.loader: instead of staying outside of the if statement as it was?

def run_flash_cmd(self, fname, flash_file, **kwargs):
loader_details = ""
if self.supports_loader and self.loader:
loader_details = "?" + self.loader
cmd = (
[self.commander]
+ (
['-IP', f'{self.dev_id}']
if (is_ip(self.dev_id) or is_tunnel(self.dev_id))
else (['-USB', f'{self.dev_id}'] if self.dev_id else [])
)
+ (['-nogui', '1'] if self.supports_nogui else [])
+ ['-if', self.iface]
+ ['-speed', self.speed]
+ ['-device', self.device + loader_details]
+ ['-CommanderScript', fname]
+ (['-nogui', '1'] if self.supports_nogui else [])
+ self.tool_opt
)
if flash_file:
self.logger.info(f'Flashing file: {flash_file}')
kwargs = {}
if not self.logger.isEnabledFor(logging.DEBUG):
kwargs['stdout'] = subprocess.DEVNULL
self.check_call(cmd, **kwargs)

west flash command stops to works at my side because variable cmd is not initialized, as the loader option is not used.
Hence causing exception to the thrown at the execution line at L438:

self.check_call(cmd, **kwargs)

#82005

@jcpi-q @pdgendt can you help line up a (hot)fix please? Or we can just revert for now.

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

Successfully merging this pull request may close these issues.

6 participants