Feat/status bar by wyzula-jan · Pull Request #966 · bec-project/bec_widgets

wyzula-jan · 2025-12-01T12:18:15Z

Description

Provides a status bar pill like widget based on beamline conditions messages/endpoints introduced in bec-project/bec#676 .

Related Issues

-closes #963

Type of Change

Added widget and action for toolbar for BL Status
New StatusToolBar
Headless Singleton Status Broker distributing status updates and newly defined statuses by BEC

How to test

Run unit tests
Define BL Condition in CLI:

• default@bec [3/18] ❯❯ samx_condition = DeviceWithinLimitsCondition(name='samx_limits',title="SamX")
• default@bec [4/18] ❯❯ samx_condition.configure(device="samx",min_limit=-10,max_limit=10,signal="samx")
• default@bec [5/18] ❯❯ bec.beamline_conditions.add(samx_condition)

The new pill should show automatically in the GUI Main Window in bottom left
try to move the motor outside the limits to see the pill status change and check tooltip

Potential side effects

...

Screenshots / GIFs (if applicable)

…e widget recursively

…rsively

… settings wip widget state manager saving loading file logic

…erated

… xcb

…init to prevent segfault

codecov · 2025-12-01T12:23:14Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

This PR adds a status bar feature to views with LED-style indicators that can display different states using color-coded visual feedback. The feature enables views to show status information (e.g., "Ready", "Processing", "Error") with visual indicators that integrate with the application's theme system.

Key changes:

Introduces StatusIndicatorWidget and StatusIndicatorAction classes for displaying LED-style status indicators
Extends ViewBase with optional status toolbar functionality controlled via show_status parameter
Exposes the QApplication instance in the Jupyter console for theme/status testing

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
bec_widgets/utils/toolbars/actions.py	Adds `StatusIndicatorWidget` (visual LED+label) and `StatusIndicatorAction` (toolbar integration) with theme-aware color resolution
bec_widgets/applications/views/view.py	Extends `ViewBase` with status toolbar initialization and management methods (`add_status_item`, `set_status`, etc.)
bec_widgets/examples/jupyter_console/jupyter_console_window.py	Exposes QApplication instance to console for easier theme and status testing

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copilot · 2025-12-01T12:24:05Z

bec_widgets/utils/toolbars/actions.py

+class StatusIndicatorWidget(QWidget):
+    """Small LED + label widget that reflects a view's status."""
+
+    def __init__(self, parent=None, text: str = "Ready", state: str = "default"):


The state parameter lacks documentation about what valid values are expected. Consider adding a docstring that documents the expected state values (e.g., "default", "success", "error", "warning") or the mechanism by which states map to colors via AccentColors.

Suggested change

def __init__(self, parent=None, text: str = "Ready", state: str = "default"):

def __init__(self, parent=None, text: str = "Ready", state: str = "default"):

"""

Initialize the StatusIndicatorWidget.

Args:

parent: The parent widget.

text (str): The label text to display next to the LED.

state (str): The status state, which determines the LED color.

Valid values are typically "default", "success", "error", "warning",

or any string key that maps to a color in AccentColors.

The mapping of state values to colors is handled via AccentColors.

"""

Copilot · 2025-12-01T12:24:06Z

bec_widgets/utils/toolbars/actions.py

+
+    def _apply_state(self, state: str):
+        palette = self._resolve_accent_colors()
+        color = getattr(palette, state, None) or getattr(palette, "default", QColor("gray"))


The use of getattr(palette, state, None) or getattr(palette, "default", QColor("gray")) could silently mask issues if an invalid state is provided. Consider logging a warning when a state is not found in the palette, or raising an exception for invalid states during development to make debugging easier.

Suggested change

color = getattr(palette, state, None) or getattr(palette, "default", QColor("gray"))

color = getattr(palette, state, None)

if color is None:

logger.warning(

f"StatusIndicatorWidget: Invalid state '{state}' for text '{self._text}'. "

"Falling back to default color."

)

color = getattr(palette, "default", QColor("gray"))

Copilot · 2025-12-01T12:24:06Z

bec_widgets/utils/toolbars/actions.py

+    """Toolbar action hosting a LED indicator and status text."""
+


The state parameter lacks documentation about what valid values are expected. Consider documenting the expected state values or referencing how they map to colors via AccentColors.

Suggested change

"""Toolbar action hosting a LED indicator and status text."""

"""

Toolbar action hosting a LED indicator and status text.

Args:

text (str, optional): The status text to display. Defaults to "Ready".

state (str, optional): The state of the indicator, which determines its color.

Valid values are keys from `AccentColors`, such as "default", "success", "warning", "error", etc.

The state value maps to a color via `AccentColors`.

tooltip (str, optional): The tooltip for the action. Defaults to "View status".

"""

Copilot · 2025-12-01T12:24:06Z

bec_widgets/applications/views/view.py

        *,
        id: str | None = None,
        title: str | None = None,
+        show_status: bool = True,


The new show_status parameter is not documented in the class docstring above (lines 65-74). Consider updating the docstring to describe this parameter and its effect on the view.

bec_widgets/applications/views/view.py

…uick debugging

…show launcher

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-08T10:41:35Z

bec_widgets/utils/toolbars/status_bar.py

+    def __new__(cls, *args, **kwargs):
+        if cls._instance is None:
+            cls._instance = super().__new__(cls)
+        return cls._instance
+
+    def __init__(self, parent=None, gui_id: str | None = None, client=None, **kwargs):
+        if self._initialized:
+            return
+        super().__init__(parent=parent, gui_id=gui_id, client=client, **kwargs)
+        self._watched: set[str] = set()
+        self.bec_dispatcher.connect_slot(
+            self.on_available, MessageEndpoints.available_beamline_conditions()
+        )
+
+        self._initialized = True
+        self.refresh_available()


The singleton pattern implementation has a potential thread-safety issue. If multiple threads call __new__ simultaneously, they could both see _instance as None and create multiple instances. Similarly, the __init__ check on _initialized could allow multiple initializations. Consider using a lock (e.g., threading.Lock()) to ensure thread-safe singleton creation, or use Python's @functools.lru_cache or threading.local() if appropriate for your use case.

Copilot · 2025-12-08T10:41:35Z

bec_widgets/widgets/containers/main_window/main_window.py

+        if hasattr(self, "_app_id_action"):
+            self._app_id_action.setText(status_message)


The hasattr(self, "_app_id_action") check is unnecessary since _app_id_action is always created in _setup_menu_bar() which is called during initialization. This check adds complexity without clear benefit. Consider either: (1) removing the hasattr check if you're confident the attribute always exists when this method is called, or (2) initializing _app_id_action to None earlier in __init__ to make it a guaranteed instance attribute.

Suggested change

if hasattr(self, "_app_id_action"):

self._app_id_action.setText(status_message)

self._app_id_action.setText(status_message)

Copilot · 2025-12-08T10:41:36Z

bec_widgets/utils/toolbars/status_bar.py

+            if self.allowed_names is None or name in self.allowed_names:
+                self.add_status_item(name=name, text=title, state=StatusState.DEFAULT, tooltip=None)
+            else:
+                # keep hidden but present for context menu toggling
+                self.add_status_item(name=name, text=title, state=StatusState.DEFAULT, tooltip=None)


The same add_status_item call is duplicated in both branches of the if-else statement (lines 163 and 166). The only difference is whether the action is set to visible afterwards. Consider refactoring this to call add_status_item once before the conditional, then only set visibility inside the else block. This would reduce duplication and make the code easier to maintain.

Suggested change

if self.allowed_names is None or name in self.allowed_names:

self.add_status_item(name=name, text=title, state=StatusState.DEFAULT, tooltip=None)

else:

# keep hidden but present for context menu toggling

self.add_status_item(name=name, text=title, state=StatusState.DEFAULT, tooltip=None)

self.add_status_item(name=name, text=title, state=StatusState.DEFAULT, tooltip=None)

if not (self.allowed_names is None or name in self.allowed_names):

# keep hidden but present for context menu toggling

Copilot · 2025-12-08T10:41:36Z

bec_widgets/utils/toolbars/status_bar.py

+        QTimer.singleShot(0, self.refresh_from_broker)
+
+    def refresh_from_broker(self) -> None:
+


There's an unnecessary blank line at the beginning of the method body. While not a functional issue, it's inconsistent with Python style conventions (PEP 8) which recommend no blank lines immediately after a function definition.

Suggested change

Copilot · 2025-12-08T10:41:36Z

bec_widgets/utils/toolbars/status_bar.py

+    def __init__(self, parent=None, gui_id: str | None = None, client=None, **kwargs):
+        if self._initialized:
+            return
+        super().__init__(parent=parent, gui_id=gui_id, client=client, **kwargs)


The singleton BECStatusBroker combines with QObject which has its own parent-based memory management. Since singletons live for the entire application lifetime, passing a parent to __init__ could lead to unexpected behavior if that parent is deleted. Consider either: (1) not accepting a parent parameter for singletons, or (2) explicitly documenting that the parent should be None or long-lived, or (3) always passing parent=None to the QObject constructor to avoid parent-based lifetime management.

Suggested change

def __init__(self, parent=None, gui_id: str | None = None, client=None, **kwargs):

if self._initialized:

return

super().__init__(parent=parent, gui_id=gui_id, client=client, **kwargs)

def __init__(self, gui_id: str | None = None, client=None, **kwargs):

if self._initialized:

return

super().__init__(parent=None, gui_id=gui_id, client=client, **kwargs)

Copilot · 2025-12-08T10:41:37Z

bec_widgets/applications/views/view.py

+        if self.status_bar is None:
+            return


The second check if self.status_bar is None: at line 134 is redundant. The setup_status_bar method at line 133 will only create a status bar if one doesn't exist (it has an early return at line 122-123 if self.status_bar is not None). After calling setup_status_bar, the status bar will either exist or will remain None (if setup failed), but the condition can never fail after calling setup when it was None before. Consider removing the redundant check or restructuring the logic for clarity.

Suggested change

if self.status_bar is None:

return

Copilot · 2025-12-08T10:41:37Z

bec_widgets/utils/toolbars/status_bar.py

+        known_actions = [
+            n for n in self.components._components.keys() if n not in ("separator",)
+        ]  # direct access used for clean-up


Direct access to self.components._components (a private attribute indicated by the leading underscore) violates encapsulation. Consider adding a public method to the components API to retrieve all component keys/names, or iterating through a public interface. If direct access is necessary for cleanup, document this exception in a comment explaining why the public API is insufficient.

Suggested change

known_actions = [

n for n in self.components._components.keys() if n not in ("separator",)

] # direct access used for clean-up

# Use public API to get component names instead of accessing private attribute

known_actions = [

n for n in self.components.get_component_names() if n not in ("separator",)

]

Copilot · 2025-12-08T10:41:38Z

bec_widgets/utils/toolbars/status_bar.py

+        self.refresh()
+
+    @SafeSlot(str, dict)
+    def on_status_updated(self, name: str, payload: dict):  # TODO finish update logic


The TODO comment "finish update logic" suggests incomplete implementation. The logic appears complete - it maps status strings to states, handles title updates, and manages tooltips. Consider removing this TODO comment if the implementation is actually complete, or add specific details about what remains to be implemented.

Suggested change

def on_status_updated(self, name: str, payload: dict): # TODO finish update logic

def on_status_updated(self, name: str, payload: dict):

Copilot · 2025-12-08T10:41:38Z

bec_widgets/utils/toolbars/actions.py

+            except Exception:
+                pass


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except Exception:

pass

except Exception as e:

logger.exception("Failed to get theme foreground color")

Copilot · 2025-12-08T10:41:38Z

bec_widgets/utils/toolbars/actions.py

+            try:
+                theme.theme_changed.connect(lambda _: self._apply_state(self._state))
+                self._theme_connected = True
+            except Exception:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except Exception:

except Exception:

# Ignore exceptions when connecting theme_changed signal.

# This may happen if the theme object does not support signal connections.

wyzula-jan and others added 30 commits October 15, 2025 14:07

build: PySide6-QtAds dependency added

bf12355

fix(bec_connector): dedicated remove signal added for listeners

e4fb126

fix(bec_connector): added name established signal for listeners

5a49c41

refactor(bec_connector): signals renamed

8f359f8

feat(widget_io): widget hierarchy can grap all bec connectors from th…

00c0232

…e widget recursively

feat(widget_io): widget hierarchy find_ancestor added

faa0aa6

refactor(widget_io): ancestor hierarchy methods consolidated

77295f1

fix(widget_state_manager): state manager can save all properties recu…

5ecdaf4

…rsively

fix(widget_state_manager): state manager can save to already existing…

a58fb3f

… settings wip widget state manager saving loading file logic

feat(bec_widget): attach/detach method for all widgets + client regen…

6643dc2

…erated

refactor(bec_main_window): main app theme renamed to View

24f54fc

feat(advanced_dock_area): added ads based dock area with profiles

cbcf239

fix(bec_widgets): by default the linux display manager is switched to…

4e96d6c

… xcb

refactor(advanced_dock_area): ads changed to separate widget

6b1b7a6

feat(advanced_dock_area): ads has default direction

9b9b8ab

fix(advanced_dock_area): dock manager global flags initialised in BW …

d4ea9f2

…init to prevent segfault

refactor(advanced_dock_area): profile tools moved to separate module

66f01dc

build(bec_qthemes): version 1.0 dependency

63b7f92

fix(bec_widgets): adapt to bec_qthemes 1.0

138fd1b

fix:queue abort button fixed

125c94c

fix(toolbar): toolbar menu button fixed

dcdf5a4

fix: device combobox change paint event to stylesheet change

b6f8866

fix: tree items due to pushbutton margins

43d4927

fix: remove pyqtgraph styling logic

03fe3a8

fix: compact popup layout spacing

daeec7e

chore: fix formatter

3e23ba9

fix(compact_popup): import from qtpy instead of pyside6

b2d6b8d

build: add missing darkdetect dependency

b029945

test: fixes after theme changes

d21464f

ci: add artifact upload

bdd732b

wyzula-jan marked this pull request as draft December 1, 2025 12:18

wyzula-jan self-assigned this Dec 1, 2025

Copilot started reviewing on behalf of wyzula-jan December 1, 2025 12:18 View session

Copilot finished reviewing on behalf of wyzula-jan December 1, 2025 12:21

Copilot AI reviewed Dec 1, 2025

View reviewed changes

wyzula-jan force-pushed the feat/status-bar branch 4 times, most recently from 09ddcab to 7d795c5 Compare December 5, 2025 15:51

fix(client): motor map docstring fixed

1210372

wyzula-jan force-pushed the feat/status-bar branch 3 times, most recently from 81c4208 to 81c2a78 Compare December 8, 2025 10:29

wyzula-jan added 4 commits December 8, 2025 11:32

fix(jupyter_console_window): added hook to current app instance for q…

c60cb31

…uick debugging

feat(status_bar): added StatusToolBar with status actions and Broker

6836036

feat(main_window): status bar added to main window bottom left

5514666

fix(launch_window): added one more connection for BECStatusBroker to …

d6f5c0e

…show launcher

wyzula-jan force-pushed the feat/status-bar branch from 81c2a78 to d6f5c0e Compare December 8, 2025 10:32

wyzula-jan requested a review from Copilot December 8, 2025 10:35

Copilot started reviewing on behalf of wyzula-jan December 8, 2025 10:36 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

wyzula-jan force-pushed the pre_release_v3 branch 7 times, most recently from c28fbf6 to b4ad75a Compare December 19, 2025 13:12

wyzula-jan force-pushed the pre_release_v3 branch from b4ad75a to 2ffe269 Compare January 5, 2026 10:32

-    def __init__(self, parent=None, text: str = "Ready", state: str = "default"):
+    def __init__(self, parent=None, text: str = "Ready", state: str = "default"):
+        """
+        Initialize the StatusIndicatorWidget.
+        Args:
+            parent: The parent widget.
+            text (str): The label text to display next to the LED.
+            state (str): The status state, which determines the LED color.
+                Valid values are typically "default", "success", "error", "warning",
+                or any string key that maps to a color in AccentColors.
+                The mapping of state values to colors is handled via AccentColors.
+        """

-        color = getattr(palette, state, None) or getattr(palette, "default", QColor("gray"))
+        color = getattr(palette, state, None)
+        if color is None:
+            logger.warning(
+                f"StatusIndicatorWidget: Invalid state '{state}' for text '{self._text}'. "
+                "Falling back to default color."
+            )
+            color = getattr(palette, "default", QColor("gray"))

		"""Toolbar action hosting a LED indicator and status text."""

-    """Toolbar action hosting a LED indicator and status text."""
+    """
+    Toolbar action hosting a LED indicator and status text.
+    Args:
+        text (str, optional): The status text to display. Defaults to "Ready".
+        state (str, optional): The state of the indicator, which determines its color.
+            Valid values are keys from `AccentColors`, such as "default", "success", "warning", "error", etc.
+            The state value maps to a color via `AccentColors`.
+        tooltip (str, optional): The tooltip for the action. Defaults to "View status".
+    """

		if hasattr(self, "_app_id_action"):
		self._app_id_action.setText(status_message)

		QTimer.singleShot(0, self.refresh_from_broker)

		def refresh_from_broker(self) -> None:

	def on_status_updated(self, name: str, payload: dict): # TODO finish update logic
	def on_status_updated(self, name: str, payload: dict):

-            except Exception:
-                pass
+            except Exception as e:
+                logger.exception("Failed to get theme foreground color")

Comments

Conversation

wyzula-jan commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Type of Change

How to test

Potential side effects

Screenshots / GIFs (if applicable)

Uh oh!

codecov bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wyzula-jan commented Dec 1, 2025 •

edited

Loading

codecov bot commented Dec 1, 2025 •

edited

Loading