-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Apply update of workflows from asset repo #84
Conversation
err := d.serialPort.Close() | ||
if err != nil { | ||
return err | ||
} |
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.
I'm not sure why we were ignoring this error, but I'd ask @cmaglie if we want to report closing errors or not.
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.
As per the pluggable monitor protocol specification, the CLOSE
command must always succeed (i.e. it must not return any error). The reason is that we cannot do much in case the serial port fails to close. I'd just ignore the error:
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.
Uhm... actually no, https://arduino.github.io/arduino-cli/0.35/pluggable-monitor-specification/#close-command I'm confused with the pluggable-discovery.
Disregard my previous comment, it's OK as you did @umbynos
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.
Fantastic, Put the green mark please 🙏
Several updates to the asset repo have been made, I've just ported them to this repo.
TODO:
AWS_ROLE_TO_ASSUME
AWS_ACCESS_KEY_ID
AWS_SECRET_ACCESS_KEY