-
Notifications
You must be signed in to change notification settings - Fork 665
feat: Plot uses subscribeMessageRange #1004
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
Merged
Merged
Changes from 25 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
6892c53
POC: PlotTwo using subscribeMessageRange
rodrigo-rodrigues-ctw 5d2961f
rework Plot Two
rodrigo-rodrigues-ctw cd34b93
refactor CurrentCustomDatasetsBuilder and IndexDatasetsBuilder
rodrigo-rodrigues-ctw 02b3f2f
remove rename config action
rodrigo-rodrigues-ctw 679a654
use subscribeMessageRange in CustomDatasetsBuilderTwo
rodrigo-rodrigues-ctw 87d8bd7
rename keyUpHandlers
rodrigo-rodrigues-ctw 00f5aa9
deduplicate types from utils
rodrigo-rodrigues-ctw c5fcd44
lint
rodrigo-rodrigues-ctw cdf4890
fix lint/tests
rodrigo-rodrigues-ctw a45fe12
refactor into helpers to reduce complexity (sonar)
rodrigo-rodrigues-ctw ddbe52c
add unit tests to useSubscribeMessageRange
rodrigo-rodrigues-ctw ca561e0
add logger test to useSubscriberMessageRange
rodrigo-rodrigues-ctw 8f12a77
apply plot two changes to plot
rodrigo-rodrigues-ctw 9009460
remove logger test in useSubscriberMessageRange
rodrigo-rodrigues-ctw 6890126
fix tests
rodrigo-rodrigues-ctw 4b69342
remove plot two
rodrigo-rodrigues-ctw 714bc86
resolve sonar issues
rodrigo-rodrigues-ctw 70d86e5
fix tests
rodrigo-rodrigues-ctw 0c39109
remove blockTopicCursor
rodrigo-rodrigues-ctw 0bddfc7
remove block mentions in indexDatasetsBuilder
rodrigo-rodrigues-ctw 47291a5
improve coverage
rodrigo-rodrigues-ctw ca327e4
lint
rodrigo-rodrigues-ctw 25559e1
address pr review comments
rodrigo-rodrigues-ctw af47405
remove unnecessary import
rodrigo-rodrigues-ctw b190c21
remove commited file by accident
rodrigo-rodrigues-ctw 2d1d519
address pr comments
rodrigo-rodrigues-ctw 1b6a822
- fix seriesKeysByTopic bug
rodrigo-rodrigues-ctw f892bc4
add handleConfig tests
rodrigo-rodrigues-ctw 4a1e5ca
refactor code
rodrigo-rodrigues-ctw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
71 changes: 71 additions & 0 deletions
71
packages/suite-base/src/components/PanelExtensionAdapter/useSubscribeMessageRange.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // SPDX-FileCopyrightText: Copyright (C) 2023-2026 Bayerische Motoren Werke Aktiengesellschaft (BMW AG)<lichtblick@bmwgroup.com> | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
|
|
||
| import { useCallback, useRef } from "react"; | ||
|
|
||
| import Logger from "@lichtblick/log"; | ||
| import { SubscribeMessageRangeArgs } from "@lichtblick/suite"; | ||
| import { useMessagePipelineGetter } from "@lichtblick/suite-base/components/MessagePipeline"; | ||
| import { | ||
| ExtensionCatalog, | ||
| useExtensionCatalog, | ||
| } from "@lichtblick/suite-base/context/ExtensionCatalogContext"; | ||
|
|
||
| import { createMessageRangeIterator } from "./messageRangeIterator"; | ||
| import { MessageConverterAlertHandler } from "./types"; | ||
|
|
||
| const log = Logger.getLogger(__filename); | ||
|
|
||
| const selectInstalledMessageConverters = (state: ExtensionCatalog) => | ||
| state.installedMessageConverters; | ||
|
|
||
| export type UseSubscribeMessageRange = (args: SubscribeMessageRangeArgs) => () => void; | ||
|
|
||
| /** | ||
| * Returns a stable callback that can be used to subscribe to a message range for a topic. | ||
| * This centralizes the logic of `unstable_subscribeMessageRange` so it can be used both by | ||
| * PanelExtensionAdapter and directly by built-in panels (e.g. Plot) without requiring migration | ||
| * to the PanelExtensionContext API. | ||
| */ | ||
| export function useSubscribeMessageRange( | ||
| emitAlert?: MessageConverterAlertHandler, | ||
| ): UseSubscribeMessageRange { | ||
| // useMessagePipelineGetter returns a stable getter, so it's safe to call it inside the callback without adding it to dependencies. | ||
| const getMessagePipelineContext = useMessagePipelineGetter(); | ||
|
|
||
| // Keep messageConverters in a ref so changing converters don't invalidate the callback. | ||
| const messageConverters = useExtensionCatalog(selectInstalledMessageConverters); | ||
| const messageConvertersRef = useRef(messageConverters); | ||
| messageConvertersRef.current = messageConverters; | ||
|
|
||
| // Similarly keep emitAlert in a ref so the caller can update it without breaking stability. | ||
| const emitAlertRef = useRef(emitAlert); | ||
| emitAlertRef.current = emitAlert; | ||
|
|
||
| return useCallback( | ||
| ({ topic, convertTo, onNewRangeIterator }: SubscribeMessageRangeArgs) => { | ||
| const { sortedTopics, getBatchIterator } = getMessagePipelineContext(); | ||
|
|
||
| const rawBatchIterator = getBatchIterator(topic); | ||
| if (!rawBatchIterator) { | ||
| return () => {}; | ||
| } | ||
|
|
||
| const { iterable: messageEventIterable, cancel } = createMessageRangeIterator({ | ||
| topic, | ||
| convertTo, | ||
| rawBatchIterator, | ||
| sortedTopics, | ||
| messageConverters: messageConvertersRef.current ?? [], | ||
| emitAlert: emitAlertRef.current, | ||
| }); | ||
|
|
||
| onNewRangeIterator(messageEventIterable).catch((err: unknown) => { | ||
| log.error("Error in useSubscribeMessageRange onNewRangeIterator:", err); | ||
| }); | ||
|
|
||
| return cancel; | ||
| }, | ||
| [getMessagePipelineContext], // getMessagePipelineContext is already stable, but listed for clarity | ||
| ); | ||
| } |
114 changes: 114 additions & 0 deletions
114
packages/suite-base/src/components/PanelExtensionAdapter/useSubscriberMessageRange.test.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| /** @jest-environment jsdom */ | ||
|
|
||
| // SPDX-FileCopyrightText: Copyright (C) 2023-2026 Bayerische Motoren Werke Aktiengesellschaft (BMW AG)<lichtblick@bmwgroup.com> | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
|
|
||
| import { renderHook, act } from "@testing-library/react"; | ||
|
|
||
| import { MessageEvent } from "@lichtblick/suite"; | ||
| import { useMessagePipelineGetter } from "@lichtblick/suite-base/components/MessagePipeline"; | ||
| import { useExtensionCatalog } from "@lichtblick/suite-base/context/ExtensionCatalogContext"; | ||
| import { BasicBuilder } from "@lichtblick/test-builders"; | ||
|
|
||
| import { createMessageRangeIterator } from "./messageRangeIterator"; | ||
| import { useSubscribeMessageRange } from "./useSubscribeMessageRange"; | ||
|
|
||
| jest.mock("@lichtblick/suite-base/components/MessagePipeline", () => ({ | ||
| useMessagePipelineGetter: jest.fn(), | ||
| })); | ||
|
|
||
| jest.mock("@lichtblick/suite-base/context/ExtensionCatalogContext", () => ({ | ||
| useExtensionCatalog: jest.fn(), | ||
| })); | ||
|
|
||
| jest.mock("./messageRangeIterator", () => ({ | ||
| createMessageRangeIterator: jest.fn(), | ||
| })); | ||
|
|
||
| const mockUseMessagePipelineGetter = useMessagePipelineGetter as jest.Mock; | ||
| const mockUseExtensionCatalog = useExtensionCatalog as jest.Mock; | ||
| const mockCreateMessageRangeIterator = createMessageRangeIterator as jest.Mock; | ||
|
|
||
| describe("useSubscribeMessageRange", () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| mockUseExtensionCatalog.mockReturnValue([]); | ||
| }); | ||
|
|
||
| it("does not call onNewRangeIterator when batch iterator is unavailable", () => { | ||
| // Given | ||
| mockUseMessagePipelineGetter.mockReturnValue( | ||
| jest.fn().mockReturnValue({ | ||
| sortedTopics: [], | ||
| getBatchIterator: jest.fn().mockReturnValue(undefined), | ||
| }), | ||
| ); | ||
| const onNewRangeIterator = jest.fn().mockResolvedValue(undefined); | ||
| const { result } = renderHook(() => useSubscribeMessageRange()); | ||
|
|
||
| // When | ||
| act(() => { | ||
| result.current({ topic: BasicBuilder.string(), onNewRangeIterator }); | ||
| }); | ||
|
|
||
| // Then | ||
| expect(onNewRangeIterator).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns a callable cancel function when batch iterator is unavailable", () => { | ||
| // Given | ||
| mockUseMessagePipelineGetter.mockReturnValue( | ||
| jest.fn().mockReturnValue({ | ||
| sortedTopics: [], | ||
| getBatchIterator: jest.fn().mockReturnValue(undefined), | ||
| }), | ||
| ); | ||
| const onNewRangeIterator = jest.fn().mockResolvedValue(async () => {}); | ||
| const { result } = renderHook(() => useSubscribeMessageRange()); | ||
| let cancel!: () => void; | ||
|
|
||
| // When | ||
| act(() => { | ||
| cancel = result.current({ topic: BasicBuilder.string(), onNewRangeIterator }); | ||
| }); | ||
|
|
||
| // Then | ||
| expect(() => { | ||
| cancel(); | ||
| }).not.toThrow(); | ||
| }); | ||
|
|
||
| it("calls onNewRangeIterator with the iterable when batch iterator is available", () => { | ||
| // Given | ||
| const topic = BasicBuilder.string(); | ||
| const mockIterable: AsyncIterable<MessageEvent[]> = { [Symbol.asyncIterator]: jest.fn() }; | ||
| const mockCancel = jest.fn(); | ||
| let cancel!: () => void; | ||
| const mockBatchIterator = { [Symbol.asyncIterator]: jest.fn() }; | ||
| const onNewRangeIterator = jest.fn().mockResolvedValue(async () => {}); | ||
| mockCreateMessageRangeIterator.mockReturnValue({ iterable: mockIterable, cancel: mockCancel }); | ||
| const mockGetBatchIterator = jest.fn().mockReturnValue(mockBatchIterator); | ||
| mockUseMessagePipelineGetter.mockReturnValue( | ||
| jest.fn().mockReturnValue({ sortedTopics: [], getBatchIterator: mockGetBatchIterator }), | ||
| ); | ||
|
|
||
| const { result } = renderHook(() => useSubscribeMessageRange()); | ||
|
|
||
| // When | ||
| act(() => { | ||
| cancel = result.current({ topic, onNewRangeIterator }); | ||
| }); | ||
|
|
||
| // Then | ||
| expect(mockGetBatchIterator).toHaveBeenCalledWith(topic); | ||
| expect(onNewRangeIterator).toHaveBeenCalledWith(mockIterable); | ||
| expect(cancel).toBe(mockCancel); | ||
| expect(mockCreateMessageRangeIterator).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| topic, | ||
| rawBatchIterator: mockBatchIterator, | ||
| sortedTopics: [], | ||
| }), | ||
| ); | ||
| }); | ||
| }); |
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.