Skip to content

Conversation

@sca075
Copy link
Owner

@sca075 sca075 commented Sep 30, 2025

We need to limit the access of this class to internal library use only, image data exported by dictionary

@sca075 sca075 self-assigned this Sep 30, 2025
@sca075 sca075 requested a review from Copilot September 30, 2025 11:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the version to v0.1.12 and refactors the CameraShared class to limit access for internal library use only, with image data now exported via dictionary.

  • Updated version from v0.1.10 to v0.1.12
  • Removed inline comments to clean up code structure
  • Refactored CameraSharedManager to use singleton pattern instead of dictionary-based instances
  • Added to_dict() method to CameraShared for exporting image data

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Added docstrings for methods in shared.py to improve code documentation and clarity.
@sca075 sca075 requested a review from Copilot September 30, 2025 11:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

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


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

sca075 and others added 6 commits September 30, 2025 13:44
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sca075 sca075 changed the base branch from main to dev September 30, 2025 11:47
@sca075 sca075 requested a review from Copilot September 30, 2025 11:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +209 to +220
def to_dict(self) -> dict:
"""Return a dictionary with image and attributes data."""
return {
"image": {
"binary": self.binary_image,
"pil_last_image": self.last_image,
"size": self.image_size,
"format": self.image_format,
"updated": self.image_last_updated,
},
"attributes": self.generate_attributes(),
}
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The new to_dict() method lacks comprehensive documentation. The docstring should explain the structure of the returned dictionary, what each key contains, and the purpose of this method for internal library use.

Copilot uses AI. Check for mistakes.

def update_user_colors(self, user_colors):
"""Update the user colors."""
"""Update user colors palette"""
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The updated docstring is incomplete and grammatically incorrect. It should be 'Update the user colors palette' or 'Update user color palette' to maintain consistency with other docstrings in the class.

Suggested change
"""Update user colors palette"""
"""Update the user colors palette."""

Copilot uses AI. Check for mistakes.

def get_user_colors(self):
"""Get the user colors."""
"""Return user colors"""
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The docstring should be more descriptive. Consider changing it to 'Return the user colors palette' to be consistent with the update method and provide clearer context.

Suggested change
"""Return user colors"""
"""Return the user colors palette"""

Copilot uses AI. Check for mistakes.

def get_rooms_colors(self):
"""Get the rooms colors."""
"""Return rooms colors"""
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The docstring should be more descriptive and grammatically correct. Consider changing it to 'Return the rooms colors palette' to maintain consistency with other color-related methods.

Suggested change
"""Return rooms colors"""
"""Return the rooms colors palette"""

Copilot uses AI. Check for mistakes.

async def batch_update(self, **kwargs):
"""Batch update multiple attributes."""
"""Update the data of Shared in Batch"""
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The docstring has awkward capitalization and phrasing. It should be 'Update shared data in batch' or 'Batch update multiple attributes' (as it was originally) for better clarity and consistency.

Suggested change
"""Update the data of Shared in Batch"""
"""Batch update multiple attributes."""

Copilot uses AI. Check for mistakes.
@sca075 sca075 merged commit 068b58e into dev Sep 30, 2025
2 checks passed
@sca075 sca075 deleted the sca075-shared branch September 30, 2025 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants