-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
twister: set serial option as required #41170
Conversation
Set serial option in hardware map as required - it is necessary to connect with hardware device. If this is achived - it seems to no sense to check in device_is_available function if serial or serial_pty is not set (some of them have to be set previously). Fixes: 41169 Signed-off-by: Piotr Golyzniak <[email protected]>
@@ -29,7 +29,7 @@ sequence: | |||
required: true | |||
"serial": | |||
type: str | |||
required: false | |||
required: true |
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.
how does this solve the problem? you can still provide an empty string in the map file and end up with the same issue. You also need to check for valid values, solving this on the schema is not enough.
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, this is not enough to solve this problem. This will work only if (d.serial or d.serial_pty)
checking will be removed from device_is_available
method from DeviceHandler
class.
If it will be provided, that serial will be required in hardware map, than it will be unnecessary to check (d.serial or d.serial_pty)
in device_is_available
method from DeviceHandler
class. Due to removing this checking, when empty string as serial port will be passed, then program will not get stuck in endless while loop in handle
method, but go forward and during trying to create Serial
object (with empty string as a path argument) program will rise an error.
@@ -683,7 +683,7 @@ def device_is_available(self, instance): | |||
for d in self.suite.duts: | |||
if fixture and fixture not in d.fixtures: | |||
continue | |||
if d.platform != device or not (d.serial or d.serial_pty): |
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.
why is this check being removed?
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
@mbolivar-nordic thank you for reminder :) I came to the conclusion, that I will try new approach here: #43963 and additionally I attach better description about how my proposition could solve problem written in #41169 issue. |
Set serial option in hardware map as required - it is necessary to
connect with hardware device. If this is achived - it seems to no sense
to check in device_is_available function if serial or serial_pty is not
set (some of them have to be set previously).
This solution help to detect and rise error in situation when passed
serial option is empty string.
Fixes: #41169
Signed-off-by: Piotr Golyzniak [email protected]