makefiles/tools/serial: Check empty port

[jcarrano]

Please note, the github UI shows commits in the wrong order.

Contribution description

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

Remarks

Hopefully this serves to illustrate why gratuitous usage of export and := should be avoided in makefiles.

I'm thinking on putting these kind of helper functions in a separate utils.mk modules.

Testing procedure

Try to make term for BOARD=cc2538dk without any connected board and see how it fails. You should still be able to make with no problems or warnings.

Issues/PRs references

Parallel to #7695.
Contains some changes by (and made on request of, and with assistance of) @cladmi
Depends on #10439, #10440

[cladmi]

This is only part of the required cleanup but should show what should be done to only evaluate PORT when needed without scarifying that an empty port produces an error.
It is currently wip as everything is defined in the same file and not documented/tested.

@gebart would that be an adequate direction as a base for your PR ?

[cladmi]

For a detailed testing procedure:

Test with the cc2538dk board to do make clean and make term.

With this PR

No check when term is not required (used clean for a shorter output but is the same with all)

BOARD=cc2538dk make --no-print-directory -C examples/hello-world/ clean
# no output here

and an error message on term

BOARD=cc2538dk make --no-print-directory -C examples/hello-world/ term
/home/harter/work/git/RIOT/examples/hello-world/../../Makefile.include:529: *** No port set.  Stop.

In master

You get a warning when not needed

 BOARD=cc2538dk make --no-print-directory -C examples/hello-world/ clean
Warning: no PORT set!

The warning and a crash when termprog is executed

BOARD=cc2538dk make --no-print-directory -C examples/hello-world/ term
Warning: no PORT set!
/home/harter/work/git/RIOT/dist/tools/pyterm/pyterm -p "" -b "115200"
No handlers could be found for logger "root"
No port specified, using default (/dev/ttyUSB0)!
2018-11-07 15:15:10,600 - INFO # Connect to serial port /dev/ttyUSB0
Traceback (most recent call last):
  File "/home/harter/work/git/RIOT/dist/tools/pyterm/pyterm", line 815, in <module>
    myshell.cmdloop("Welcome to pyterm!\nType '/exit' to exit.")
  File "/usr/lib/python2.7/cmd.py", line 109, in cmdloop
    self.preloop()
  File "/home/harter/work/git/RIOT/dist/tools/pyterm/pyterm", line 245, in preloop
    self.serial_connect()
  File "/home/harter/work/git/RIOT/dist/tools/pyterm/pyterm", line 611, in serial_connect
    self.ser = serial.Serial(port=self.port, dsrdtr=0, rtscts=0)
  File "/usr/lib/python2.7/dist-packages/serial/serialutil.py", line 240, in __init__
    self.open()
  File "/usr/lib/python2.7/dist-packages/serial/serialposix.py", line 268, in open
    raise SerialException(msg.errno, "could not open port {}: {}".format(self._port, msg))
serial.serialutil.SerialException: [Errno 2] could not open port /dev/ttyUSB0: [Errno 2] No such file or directory: '/dev/ttyUSB0'
/home/harter/work/git/RIOT/examples/hello-world/../../Makefile.include:529: recipe for target 'term' failed
make: *** [term] Error 1

[cladmi]

Could you try adding this diff and run a build in CI. It should make murdock fail if one board uses PORT.

diff --git a/.murdock b/.murdock
index 5d03a2a09..02690805e 100755
--- a/.murdock
+++ b/.murdock
@@ -153,6 +153,11 @@ compile() {
     [ ! -d "$appdir" ] && error "$0: compile: error: application directory \"$appdir\" doesn't exist"
     [ ! -d "boards/$board" ] && error "$0: compile: error: board directory \"boards/$board\" doesn't exist"
 
+    # Try removing PORT that should be already be set by murdock
+    #     https://github.com/RIOT-OS/RIOT/pull/7695#discussion_r180896514
+    # to let the auto-detection work and fail in murdock
+    unset PORT
+
     # compile
     CCACHE_BASEDIR="$(pwd)" BOARD=$board TOOLCHAIN=$toolchain RIOT_CI_BUILD=1 \
         make -C${appdir} clean all -j${JOBS:-4}

I cannot directly push to your branch.

[cladmi]

The ensure_value could be put in a common makefiles/utils/variables.inc.mk as there are other places that would be benefit from it like the wget/curl handling and unzip|7z.

I also have other helper functions that I would want to but in common.
Same as the thin-archives functions.

[cladmi]

To help getting this merged, I would keep this PR as a global PR with split ones.
The last step being doing the PORT ?= $(call ensure_value,$(PORT_LINUX),No port set) and removing the empty port check.

I cen start reviewing PRs that removes exports as they can be integrated alone.
Also a PR for the ensure_value alone is easier to review.

[jcarrano]

I cherry-picked #10454 inside this PR. Hope murdock is happy now.

[jcarrano]

@cladmi I rebased and most of the PR disappeared (it's a good thing, means stuff got merged). I removed the WIP tag, I think this is not WIP any more.

[jcarrano]

D'oh! I screwed up the rebase, force pushing again.

[smlng]

not waiting anymore as #10440 is merged.

[aabadie]

There's a REMOVEME DELETEME commit in this PR. Blocking to avoid any accidental merge.

[fjmolinas]

The PR needs rebasing. Most of the commits are already in.

[aabadie]

Most of the commits are already in.

True. c103b3f is merged already.

[fjmolinas]

True. c103b3f is merged already.

Only the commit I cherry-picked is missing.

[aabadie]

Only the commit I cherry-picked is missing.

Yes that's what I just noticed as well. I propose we close this PR and go with #12479.