Skip to content
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

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Conversation

jmo-wp
Copy link
Contributor

@jmo-wp jmo-wp commented Oct 9, 2024

Improve it to support:

  • gateway list from file
  • logging to file
  • generic usage of OTAP with delay action (no hardcoded value)
  • control of its behavior via the strict mode switch

Copy link
Contributor

@LePailleurThibault LePailleurThibault left a 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.

examples/example_otap.py Outdated Show resolved Hide resolved
examples/example_otap.py Show resolved Hide resolved
@jmo-wp jmo-wp force-pushed the feature/improve-example-otap-script branch 2 times, most recently from 165a6e0 to 51c8697 Compare October 9, 2024 09:35
@LePailleurThibault
Copy link
Contributor

Good to me

Copy link
Contributor

@GwendalRaoul GwendalRaoul left a 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 = [
Copy link
Contributor

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 :-)

Copy link
Contributor Author

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.

@jmo-wp jmo-wp force-pushed the feature/improve-example-otap-script branch from 51c8697 to adeb959 Compare October 24, 2024 12:56
@jmo-wp jmo-wp force-pushed the feature/improve-example-otap-script branch from adeb959 to 2c4dfe3 Compare October 24, 2024 12:59
@jmo-wp jmo-wp merged commit 1657db1 into main Oct 24, 2024
2 checks passed
@jmo-wp jmo-wp deleted the feature/improve-example-otap-script branch October 24, 2024 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants