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

Extend spec with publishing time data #299

Merged
merged 4 commits into from
Dec 14, 2022

Conversation

achim-k
Copy link
Collaborator

@achim-k achim-k commented Dec 13, 2022

Public-Facing Changes

  • Allow server to publish time data (binary)
  • Adapt typescript client/server accordingly

Description

  • Extends the spec by adding publishing of time data under a new opcode (0x02). By publishing time data, clients can be informed about the progress of time even if no other message data is published.
  • Updates @foxglove/ws-protocol accordingly

Supersedes #298

docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
docs/spec.md Outdated Show resolved Hide resolved
@achim-k achim-k force-pushed the achim/spec_time_data branch 2 times, most recently from 3609b40 to 5c26bca Compare December 13, 2022 21:47
@achim-k
Copy link
Collaborator Author

achim-k commented Dec 13, 2022

@jtbandes @jhurliman I have now also added the necessary adaptation to @foxglove/ws-protocol to this PR. Let me know if I should rather do open a new PR for that

Copy link
Member

@jtbandes jtbandes left a comment

Choose a reason for hiding this comment

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

I don't mind having it in the same PR once we are aligned on the semantics.

typescript/ws-protocol/src/FoxgloveServer.ts Outdated Show resolved Hide resolved
typescript/ws-protocol/src/types.ts Outdated Show resolved Hide resolved
typescript/ws-protocol/src/FoxgloveServer.ts Outdated Show resolved Hide resolved
@jhurliman jhurliman merged commit c98cbfe into foxglove:main Dec 14, 2022
@achim-k achim-k deleted the achim/spec_time_data branch December 14, 2022 19:10
jhurliman pushed a commit to foxglove/ros-foxglove-bridge that referenced this pull request Dec 14, 2022
**Public-Facing Changes**
- Publish binary time data when `use_sim_time` is `true`

**Description**
Depends on foxglove/ws-protocol#299

- When the `use_sim_time` parameter is set
  - The server advertises the `time` capability
- It subscribes to the `/clock` topic and broadcasts binary time
messages
- I have removed the code that detects presence of the `/clock` topic,
since now it has to be known at startup whether `use_sim_time` is true
or false

- Includes #115 (will rebase after it got merged)
pezy pushed a commit to pezy/ws-protocol that referenced this pull request Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants