Skip to content

makefiles: Provide USB UART device serial number matching #7695

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 14 commits into from
Closed

makefiles: Provide USB UART device serial number matching #7695

wants to merge 14 commits into from

Conversation

jnohlgard
Copy link
Member

@jnohlgard jnohlgard commented Oct 8, 2017 •
edited
Loading

Based on #7686
In the present master branch, each board provides its own way of guessing the name of any USB to UART adapter to use for make term by setting the PORT environment variable. This PR centralizes this in makefiles/tools/serial.inc.mk and adds USB serial number matching. If PORT is given, then nothing changes, the make term command will still use the port specified.
Otherwise, if USB_UART_ID is given as an environment variable, then that string is searched for as a USB serial number. If no USB_UART_ID is given, fall back to using DEBUG_ADAPTER_ID instead. Last resort is to pick the first found USB tty device.

All USB serial device scanning is handled by find-tty.sh on Linux, and a wildcard matching method on Darwin (Mac OS)

@jnohlgard jnohlgard added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: build system Area: Build system State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Oct 8, 2017
@jnohlgard jnohlgard added the State: waiting for other PR State: The PR requires another PR to be merged first label Oct 10, 2017
@jnohlgard jnohlgard force-pushed the pr/usb-serial-handling branch 2 times, most recently from fad01b4 to 27a6593 Compare October 13, 2017 11:52
@jnohlgard jnohlgard force-pushed the pr/usb-serial-handling branch from 27a6593 to b92916a Compare October 19, 2017 14:13
@jnohlgard jnohlgard changed the title [WIP] makefiles: Provide USB UART device serial number matching makefiles: Provide USB UART device serial number matching Oct 19, 2017
@jnohlgard jnohlgard removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Oct 19, 2017
@jnohlgard
Copy link
Member Author

It would be good to get this merged early in this release cycle to avoid rebase hell with new boards being added.

@smlng
Copy link
Member

smlng commented Nov 13, 2017

please rebase, and question: is it necessary that this is based on #7686? Would make merging easier if it wasn't.

@jnohlgard jnohlgard force-pushed the pr/usb-serial-handling branch from b92916a to 2c8cbf1 Compare November 13, 2017 14:01
@jnohlgard
Copy link
Member Author

The reason is that this and #7686 both touch approximately the same files, so it will be less work if we merge them in sequence.

@jnohlgard jnohlgard force-pushed the pr/usb-serial-handling branch from 2c8cbf1 to 5eee0f4 Compare November 28, 2017 10:45
@jnohlgard jnohlgard added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Nov 28, 2017
@jnohlgard
Copy link
Member Author

rebased after dependencies were merged

@jnohlgard jnohlgard mentioned this pull request Jan 30, 2018
Closed
@jnohlgard
Copy link
Member Author

needs rebase...

@jnohlgard jnohlgard mentioned this pull request Jan 30, 2018
Closed
@jnohlgard jnohlgard force-pushed the pr/usb-serial-handling branch from 5eee0f4 to 76c052e Compare January 31, 2018 07:42
cladmi added 4 commits June 18, 2018 15:17
@cladmi
serial: remove unneccesary exports
396d2ef 
There variables are only used directly in the main Makefile.include or in
application makefiles. So there is no need to export the value.

Not exporting will also prevent evaluating them for no reason when 'term' is not
done.
@cladmi
serial: do not check for empty PORT
2db85e9 
It remove PORT evaluation when it is not used, which will lead to not calling
`find-tty.sh` in upcoming commits.
@cladmi
serial: use deferred assignment for PORT
7eb4d7d 
With deferred assignment, `find-tty.sh` will only be called when `PORT` is
evaluated when `TERMFLAGS` is evaluated for `make term`
@cladmi
serial: remove remaining useless immediate assignment
eff92c5
@cladmi
Copy link
Contributor

cladmi commented Jun 18, 2018

Done. I tested by adding echo $0 >&2 ; read DUMMY in find-tty.sh and commenting PORT_LINUX in boards/common/iotlab/Makefile.include (I only have an iotlab-m3 with me).

make term correctly blocks once on the read, and make does not.
With make info-build it is called two times, but that is normal.

I currently only removed the PORT empty warning and did not re-integrate it anywhere.

@cladmi
Copy link
Contributor

cladmi commented Jun 18, 2018

I pushed here for simplicity, but feel free to amend/squash my commit.

@jnohlgard
Copy link
Member Author

@cladmi thanks for improving this PR. I am on holidays and will not have time to work on this for some time

@cladmi
Copy link
Contributor

cladmi commented Jul 17, 2018

I tried to check if the script returning an error was handled but it looks like it is silently ignored by make.

I am thinking about echoing something like "/find-tty.sh/could/not/find/port/for/board/serial_pattern" to make it obvious when it will fails during make term. What do you think ?

Also I plan to split this PR, the common code and then convert boards in separate PRs to not get blocked on edge cases.

It will be after the release though I think.

@cladmi cladmi mentioned this pull request Aug 7, 2018
Closed
@kYc0o
Copy link
Contributor

kYc0o commented Aug 28, 2018

How is it going here?

@cladmi
Copy link
Contributor

cladmi commented Nov 6, 2018

More discussions with @jcarrano and he showed me magic trick to get an error, I did not thought about using call to get an error at execution last time I checked this PR.

https://github.com/jcarrano/RIOT/blob/36d29787e7b109e079d2f521407ec309056a8580/makefiles/utils.inc.mk#L18-L23

This could be the perfect trick to print an error when port is empty.
He will try something around this as a generic function in the build system to use it here as a base.

@jcarrano jcarrano mentioned this pull request Nov 6, 2018
Closed
@cladmi cladmi mentioned this pull request Jan 16, 2019
Merged
@cladmi cladmi mentioned this pull request Jan 23, 2019
Closed
19 tasks
@fjmolinas fjmolinas mentioned this pull request Oct 17, 2019
Merged
@stale
Copy link

stale bot commented Jan 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Jan 17, 2020
@stale stale bot closed this Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 left review comments

@aabadie aabadie aabadie left review comments

@kaspar030 kaspar030 kaspar030 requested changes

+1 more reviewer

@cladmi cladmi cladmi left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@kYc0o kYc0o

Labels
Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: stale State: The issue / PR has no activity for >185 days Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.