-
Notifications
You must be signed in to change notification settings - Fork 11
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
Improve example OTAP script #45
Conversation
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.
Minor changes should be done, but otherwise it looks good.
165a6e0
to
51c8697
Compare
Good to me |
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.
Good for me too!
You will have to update the CI to be able to merge...
It should be straight forward by bumping some actions version. Done in some other repo.
@@ -7,6 +7,27 @@ | |||
import wirepas_mesh_messaging as wmm | |||
import logging | |||
from random import choice | |||
import sys | |||
|
|||
otap_delay_choices = [ |
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.
Initially I didn't really give it as an option because I'm not sure it is clear for our customer which value to choose.
But if you gave good answer to give when someone will ask if it must use 10 or 30 minutes, go for it :-)
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.
No clear guidelines to give on this point unfortunately but as this script starts to look more like a reference one rather than an example leaving the delay hardcoded feels a bit weird.
51c8697
to
adeb959
Compare
…eneric usage of OTAP with delay action
adeb959
to
2c4dfe3
Compare
Improve it to support: