Update zenoh and enable shared memory#3953
Merged
patrickelectric merged 9 commits intoJun 24, 2026
Merged
Conversation
a953ca8 to
074e56b
Compare
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The custom
RecvErr.MalformedReplyenum value is never produced byreceiveQueryReply, but some callers (e.g.CloudTrayMenu) branch on it; consider either implementing malformed reply detection or removing that enum value and the associated branches to avoid dead code paths. - Several places duplicate the same "create query receiver, iterate replies until disconnected" pattern (e.g. in
ZenohNetwork.vue,CloudTrayMenu.vue, andlibs/zenoh/index.ts); consider extracting a shared helper to centralize this logic and keep future zenoh API migrations confined to one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The custom `RecvErr.MalformedReply` enum value is never produced by `receiveQueryReply`, but some callers (e.g. `CloudTrayMenu`) branch on it; consider either implementing malformed reply detection or removing that enum value and the associated branches to avoid dead code paths.
- Several places duplicate the same "create query receiver, iterate replies until disconnected" pattern (e.g. in `ZenohNetwork.vue`, `CloudTrayMenu.vue`, and `libs/zenoh/index.ts`); consider extracting a shared helper to centralize this logic and keep future zenoh API migrations confined to one place.
## Individual Comments
### Comment 1
<location path="core/frontend/src/libs/zenoh/index.ts" line_range="5-7" />
<code_context>
+
+// zenoh-ts 1.9 no longer exports Receiver/RecvErr; the get() channel rejects receive() on close
+// (and the query has its own timeout), so we just map that rejection to Disconnected.
+export enum RecvErr {
+ Disconnected,
+ MalformedReply,
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** RecvErr.MalformedReply is never produced by receiveQueryReply, making callers’ MalformedReply handling unreachable.
`receiveQueryReply()` currently only returns `Reply` or `RecvErr.Disconnected` (on rejection), so there’s no path that yields `RecvErr.MalformedReply`. Callers like `CloudTrayMenu.queryFileSyncQueue` that branch on `MalformedReply` are therefore dead code. Please either wire malformed replies through to `RecvErr.MalformedReply` (e.g., via parsing error handling in `receiveQueryReply`) or remove `MalformedReply` and the unused branches to keep the enum and its usage consistent.
</issue_to_address>
### Comment 2
<location path="core/frontend/src/components/cloud/CloudTrayMenu.vue" line_range="461-462" />
<code_context>
- let reply = await receiver.receive()
+ let reply = await receiveQueryReply(receiver)
while (reply !== RecvErr.Disconnected) {
if (reply === RecvErr.MalformedReply) {
console.warn(`[CloudTrayMenu] Malformed reply while querying ${topic}.`)
} else {
const response = reply.result()
</code_context>
<issue_to_address>
**issue (bug_risk):** MalformedReply branch appears unreachable given current receiveQueryReply implementation.
Since `receiveQueryReply` only returns a `Reply` or `RecvErr.Disconnected`, this `MalformedReply` case is currently unreachable and malformed replies will instead be handled by the outer `catch`. Once you decide whether `receiveQueryReply` should throw on malformed input or return `RecvErr.MalformedReply`, please align this branch with that behavior or remove it.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This comment was marked as resolved.
This comment was marked as resolved.
8b2a761 to
40a8d51
Compare
| handler: (sample: Sample) => { | ||
| this.handleUploadingSample(sample) | ||
| }, | ||
| history: true, |
Member
Author
There was a problem hiding this comment.
We could keep it, but this was already dead code in 1.3.4.
a7d61c9 to
bb2e75b
Compare
Member
Author
Member
Member
patrickelectric
requested changes
Jun 22, 2026
Collaborator
df9781b to
ebfa91b
Compare
nicoschmdt
approved these changes
Jun 22, 2026
withSchema returns a new Encoding (it does not mutate), so the schema must be captured here
… ready callback makeEvents is undefined; the ready callback always threw.
…ainst Vue reactivity Vue 2 deep-observed the Core; fcose per-frame mutations froze the tab
ebfa91b to
4c90067
Compare
Member
Author
|
Rebased. |
patrickelectric
approved these changes
Jun 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR:
1.9.0.Note: If we want zenohd to use shared memory, I might need to build it ourselves.