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

tools/cpy2remed: add support for NOD_xxxx removable media #18824

Merged
merged 1 commit into from
Nov 18, 2022

Conversation

krzysztof-cabaj
Copy link
Contributor

Contribution description

Newly manufactured Nucleo boards use NOD_xxxx removable media name instead of older NODE_xxxx. The rest of name remains the same. For example newly purchased F429 have name NOD_F429ZI, while older one NODE_F429ZI.

This PR adds to cpy2remed programmer ability to program both types of removable media names.

Testing procedure

Try flashing Nucleo board with new removable media name - NOD_xxxx . Without this PR process failed.

With this PR you could program it, as well as, other boards with NODE_xxx names.

Issues/PRs references

None

@github-actions github-actions bot added the Area: tools Area: Supplementary tools label Oct 31, 2022
@krzysztof-cabaj
Copy link
Contributor Author

Can we move this PR a little forward ;)

@aabadie @benpicco @maribu sorry for bothering but you were supporting my other PR related to cpy2remed.

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

static-tests has some comments

dist/tools/cpy2remed/cpy2remed.sh Outdated Show resolved Hide resolved
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

some style suggestions inline. Also note that you are mixing tabs and spaces in the file. Please use four spaces for indent for consistence within RIOT.

dist/tools/cpy2remed/cpy2remed.sh Outdated Show resolved Hide resolved
dist/tools/cpy2remed/cpy2remed.sh Outdated Show resolved Hide resolved
@krzysztof-cabaj
Copy link
Contributor Author

I have to replace inline bash substitute to sed, but now script works using sh.
I tested script and it now works with older and newest Nucleo.
All commits squashed.

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Looks OK to me.
I don't have a board to test this and it's a bit noisy for my taste, but other programmers produce a lot more output, so people are probably used to that 😉

@krzysztof-cabaj
Copy link
Contributor Author

@benpicco Thanks, once again!

Can we send it to Murdock? Or we wait for @maribu?

Sorry for the rush - but I cannot program new boards in the lab.

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs labels Nov 18, 2022
@maribu
Copy link
Member

maribu commented Nov 18, 2022

lgtm. The final line is missing a newline. (This shouldn't happen when the editor is properly configured. My suggestion is to check the editor config, as making sure the last line is correctly terminated is a task bette left to machines.) Also, the indent is still inconsistent. Please double check that you have 4 spaces per level of indentation.

Maybe we should provide some setting files for the most common editors. I think VS code is what most people use these days, but it sucks without manually adjusting the config. Maybe we should just add some config to the repo, so that it works out of the box with RIOT.

@krzysztof-cabaj
Copy link
Contributor Author

I fix last line issue as well as remaining indentation mismatches. Now formatting should be OK.

@riot-ci
Copy link

riot-ci commented Nov 18, 2022

Murdock results

✔️ PASSED

4cb2ed5 tools/cpy2remed: add support for removable media name NOD_xxxx

Success Failures Total Runtime
1 0 1 01m:04s

Artifacts

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

@benpicco benpicco merged commit 3c7c48e into RIOT-OS:master Nov 18, 2022
@kaspar030 kaspar030 added this to the Release 2023.01 milestone Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants