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

makefiles/tools/serial: ensure PORT is set and fail early. #12479

Merged
merged 1 commit into from
Oct 17, 2019

Conversation

fjmolinas
Copy link
Contributor

@fjmolinas fjmolinas commented Oct 17, 2019

Contribution description

This commit has been cherry-picked from #10342

By ensuring the PORT auto-detection worked, we can give meaningful
error messages and fail earlier.

This uses ensure_value from makefiles/utils/checks.mk. An include was
added to Makefile.include to make this fuction available to all other
makefiles.

This also avoids evaluating $(PORT).

Testing procedure

Set PORT_LINUX empty:

PORT_LINUX='' BOARD=cc2538dk make -C examples/hello-world/ term

make: Entering directory '/home/francisco/workspace/RIOT/examples/hello-world'
/home/francisco/workspace/RIOT/examples/hello-world/../../Makefile.include:646: *** No port set.  Stop.
make: Leaving directory '/home/francisco/workspace/RIOT/examples/hello-world'

Issues/PRs references

From #10342, closes #10342

By ensuring the PORT auto-detection worked, we can give meaningful
error messages and fail earlier.

This uses ensure_value from makefiles/utils/checks.mk. An include was
added to Makefile.include to make this fuction available to all other
makefiles.
@fjmolinas fjmolinas added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 17, 2019
@fjmolinas fjmolinas requested review from aabadie and smlng October 17, 2019 07:06
@aabadie
Copy link
Contributor

aabadie commented Oct 17, 2019

I'm not sure I understand the goal of this PR.

if I do

PORT_LINUX='' BOARD=cc2538dk make -C examples/hello-world/ term

it indeed fails early. But this is not the usual way of setting a port. The variable used in general is PORT. And if I set PORT to an empty value, then it doesn't fail early:

PORT='' BOARD=cc2538dk make -C examples/hello-world/ term
make: Entering directory '/work/riot/RIOT/examples/hello-world'
/work/riot/RIOT/dist/tools/pyterm/pyterm -p "" -b "115200" 
Twisted not available, please install it if you want to use pyterm's JSON capabilities
No port specified, using default (/dev/ttyUSB0)!
2019-10-17 10:26:23,468 # Connect to serial port /dev/ttyUSB0
2019-10-17 10:26:23,468 # Cannot connect to serial port /dev/ttyUSB0: could not open port /dev/ttyUSB0: [Errno 2] No such file or directory: '/dev/ttyUSB0'
make: *** [/work/riot/RIOT/examples/hello-world/../../Makefile.include:647: term] Error 1
make: Leaving directory '/work/riot/RIOT/examples/hello-world'

@aabadie
Copy link
Contributor

aabadie commented Oct 17, 2019

Why not just trying to merge #10342 ? (what @smlng is silently trying to do).

@fjmolinas
Copy link
Contributor Author

Why not just trying to merge #10342 ? (what @smlng is silently trying to do).

I-m fine with that.

@fjmolinas
Copy link
Contributor Author

I-m fine with that.

It was just to get the commit in faster since @jcarrano is not working on the project anymore.

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

ACK and go

@aabadie aabadie merged commit 8a877b5 into RIOT-OS:master Oct 17, 2019
@fjmolinas
Copy link
Contributor Author

@aabadie thanks for the review,
@jcarrano thanks for doing the work :)

@fjmolinas fjmolinas deleted the pr_ensure_port_is_set branch October 17, 2019 09:02
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants