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

Extended sender: simulated data #6

Merged

Conversation

miguelgarcia
Copy link
Contributor

Hey, this standard is a great initiative. We are exploring it at InOrbit since it seems like a really nice way to get data from different robots into our platform. As part of this exploration we extended the sender to be able to generate more complete status reports with changing data.

Besides this, the PR includes a couple of fixes/improvements that we believe are useful:

  • Fixed missing import of datetime timezone
  • Read URI from argv

Thanks !

@miguelgarcia
Copy link
Contributor Author

Hi @cacioppoc @zac-dydek is creating a PR the best way to contribute to this repo or is there another preferred method ?

Thanks a lot, have a great week!

Copy link
Collaborator

@zac-dydek zac-dydek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are great additions, just one small change to make the operational state choices match the standard document.


async def sendMessage():
uri = "ws://localhost:3000"
OPERATIONAL_STATES = ["navigating", "idle", "disabled", "offline", "charging", "waiting", "loadingUnloading", "manualOveride"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The operational states in our working document are similar, but not identical. We split "waiting" into:

  1. Waiting for human event (which probably covers your "loadingUnloading" state?)
  2. Waiting for external event
  3. Waiting for internal event

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @zac-dydek I copied these values from /AMR_Interop_Standard.json, which seems to don't exactly follow what's in the doc. I can piggyback that change into this PR. What do you think would be good names ?

  1. waitingHumanEvent
  2. waitingExternalEvent
  3. waitingInternalEvent
    ??

Thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, yes, it seems the .json file is slightly out-of-date. Yes, let's make that change while we're at it. The names you picked are fine with me!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @zac-dydek I made the changes, could you review ?

Thanks!

Copy link
Collaborator

@zac-dydek zac-dydek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

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.

None yet

2 participants