-
Notifications
You must be signed in to change notification settings - Fork 746
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 22 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
6892c53
POC: PlotTwo using subscribeMessageRange
rodrigo-p-rodrigues 5d2961f
rework Plot Two
rodrigo-p-rodrigues cd34b93
refactor CurrentCustomDatasetsBuilder and IndexDatasetsBuilder
rodrigo-p-rodrigues 02b3f2f
remove rename config action
rodrigo-p-rodrigues 679a654
use subscribeMessageRange in CustomDatasetsBuilderTwo
rodrigo-p-rodrigues 87d8bd7
rename keyUpHandlers
rodrigo-p-rodrigues 00f5aa9
deduplicate types from utils
rodrigo-p-rodrigues c5fcd44
lint
rodrigo-p-rodrigues cdf4890
fix lint/tests
rodrigo-p-rodrigues a45fe12
refactor into helpers to reduce complexity (sonar)
rodrigo-p-rodrigues ddbe52c
add unit tests to useSubscribeMessageRange
rodrigo-p-rodrigues ca561e0
add logger test to useSubscriberMessageRange
rodrigo-p-rodrigues 8f12a77
apply plot two changes to plot
rodrigo-p-rodrigues 9009460
remove logger test in useSubscriberMessageRange
rodrigo-p-rodrigues 6890126
fix tests
rodrigo-p-rodrigues 4b69342
remove plot two
rodrigo-p-rodrigues 714bc86
resolve sonar issues
rodrigo-p-rodrigues 70d86e5
fix tests
rodrigo-p-rodrigues 0c39109
remove blockTopicCursor
rodrigo-p-rodrigues 0bddfc7
remove block mentions in indexDatasetsBuilder
rodrigo-p-rodrigues 47291a5
improve coverage
rodrigo-p-rodrigues ca327e4
lint
rodrigo-p-rodrigues 25559e1
address pr review comments
rodrigo-p-rodrigues af47405
remove unnecessary import
rodrigo-p-rodrigues b190c21
remove commited file by accident
rodrigo-p-rodrigues 2d1d519
address pr comments
rodrigo-p-rodrigues 1b6a822
- fix seriesKeysByTopic bug
rodrigo-p-rodrigues f892bc4
add handleConfig tests
rodrigo-p-rodrigues 4a1e5ca
refactor code
rodrigo-p-rodrigues 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 | ||
| ); | ||
| } |
115 changes: 115 additions & 0 deletions
115
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,115 @@ | ||
| /** @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(typeof cancel).toBe("function"); | ||
|
rodrigo-p-rodrigues marked this conversation as resolved.
Outdated
|
||
| 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.