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/testbed: use IOTLAB_NODE=auto instead of auto-ssh #16491
tools/testbed: use IOTLAB_NODE=auto instead of auto-ssh #16491
Conversation
There's something with this PR, I'll rework it a bit. Marking it as WIP. |
22dec0b
to
8d4cf6e
Compare
I could fix the problem easily. This is not WIP anymore :) |
29c7183
to
d2325b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First comments
@@ -48,6 +48,11 @@ ifeq (,$(IOTLAB_NODE)) | |||
$(error) | |||
endif | |||
|
|||
ifeq (auto-ssh,$(IOTLAB_NODE)) | |||
$(warning IOTLAB_NODE=auto-ssh is deprecated, use IOTLAB_NODE=auto instead) | |||
override IOTLAB_NODE := auto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is override needed? I would be needed only if is set as a MAKE argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, IOTLAB_NODE is already overridden in master. Otherwise it won't work with a command like make IOTLAB_NODE=auto
@@ -48,6 +48,11 @@ ifeq (,$(IOTLAB_NODE)) | |||
$(error) | |||
endif | |||
|
|||
ifeq (auto-ssh,$(IOTLAB_NODE)) | |||
$(warning IOTLAB_NODE=auto-ssh is deprecated, use IOTLAB_NODE=auto instead) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a deprecation date?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and improved with a colored message.
d2325b8
to
b39b07d
Compare
@fjmolinas, may I squash ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thre is a reference in tests/pkg_semtech-loramac/README.md
and examples/lorawan/README.md
and .github/workflows/test-on-iotlab.yml
uses the deprecated version.
# IOTLAB_NODE=auto or auto-ssh |
Verified the testing procedure locally and on the frontend. |
Please squash once addressed. |
bed3e8d
to
d42ac48
Compare
This deprecates the use of IOTLAB_NODE=auto-ssh
d42ac48
to
12a03c6
Compare
Removed the remaining occurrences of auto-ssh and squashed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
Contribution description
This PR ticks the item in #9694 which is about merging auto and auto-ssh possible values for IOTLAB_NODE. As said by @cladmi, since there's an IOT_LAB_FRONTEND_FQDN env variable on the frontend there's no need to distinguish between auto and auto-ssh.
This makes the usage of IOTLAB_NODE more simple as well as cleaning up the internal implementation.
This PR also adds a deprecation message when auto-ssh is uses.
Testing procedure
This PR also works when the commands above are launched from an IoT-LAB SSH frontend (except the one for A8-M3 because of the bug about .bin firmwares that is fixed in version 1.0.2 of iotlab-ssh but not released on the frontends).
Issues/PRs references
Tick one item in #9694