Skip to content

update camera_set_up#9

Open
mvarentsova wants to merge 11 commits into
masterfrom
get_board_data
Open

update camera_set_up#9
mvarentsova wants to merge 11 commits into
masterfrom
get_board_data

Conversation

@mvarentsova
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Member

@AndBondStyle AndBondStyle left a comment

Choose a reason for hiding this comment

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

Общие комментарии по всему PR:

  1. Все комментарии и логи в коде пишем на английском языке
  2. Не хардкодить пути до файлов. Если хардкодить, то хотя бы относительно структуры репозитория, но лучше принимать как аргумент командной строки через argparse
  3. Не сохранять файлы (изображения, видео и т.п.) в репозиторий
  4. Код тяжело читать из-за а) полного отсутствия типизации б) из-за того что все реализовано на функциях, в которые зачастую передается один и тот же контекст (параметры камеры, координаты маркеров и т.п.) - это надо переделать на класс
  5. Весь код прогнать через линтер/форматтер, я рекомендую ruff с дефолтными настройками black + isort. А еще лучше настроить запуск ruff через утилиту pre-commit, ниже пример минимальной конфигурации, которую я обычно использую:

pyproject.toml

# ...

[tool.ruff]
line-length = 88
target-version = "py311"

[tool.ruff.lint]
select = ["I", "F", "E"]

[tool.ruff.lint.per-file-ignores]
"__init__.py" = ["F401"]

.pre-commit-config.yaml

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v5.0.0
    hooks:
      - id: check-added-large-files
      - id: check-merge-conflict
      - id: check-json
      - id: check-yaml
      - id: check-toml
      - id: end-of-file-fixer
      - id: fix-byte-order-marker
      - id: mixed-line-ending
      - id: trailing-whitespace

  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.11.9
    hooks:
      - id: ruff
        args: [--fix, --show-fixes]
      - id: ruff-format

Подробнее:

Comment thread camera_set_up/calibration.py Outdated
continue

if frame.shape[1] != TARGET_WIDTH or frame.shape[0] != TARGET_HEIGHT:
frame = cv2.resize(frame, (TARGET_WIDTH, TARGET_HEIGHT))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Так не надо делать, и в константах забивать размер кадра тоже не обязательно. Размер можно определить по первому изображению в цикле, а дальше если попадается изображение с другим размером, то просто выбрасывать ошибку

Comment thread camera_set_up/calibration.py Outdated
if aru_corners:
ok, char_corners, char_ids = cv2.aruco.interpolateCornersCharuco(aru_corners, aru_ids, gray, BOARD)

good_frame = aru_corners and char_ids is not None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Условие слабое, я бы брал кадры где хотя бы 6 маркеров нашлось

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Результат калибровки лучше хранить в yaml, писал об этом в прошлом PR. По формату файла я бы сделал как-то так:

calib_size: [width, height]
matrix: [[...], ...]
distortion: [...]

calib_size будет полезно, если в runtime будет использоваться другой размер изображения, не совпадающий с калибровкой

Comment thread get_board_data/get_board_data.py Outdated
@@ -0,0 +1,192 @@
import cv2
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Если этот файл больше не актуален, надо его удалить

return SMOOTHED_ORIGIN, SMOOTHED_X, SMOOTHED_Y, SMOOTHED_NORMAL


def GetBoardData(frame, camera_matrix, dist_coeffs, detector, left_bottom, left_top, right_top, right_bottom):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

В функции передается очень много одинакового контекста, лучше сделать из этого класс с несколькими методами. Еще хотелось бы проставить типы у аргументов

Comment thread get_board_data/get_board_data_v2.py Outdated


def SmoothBoard(origin, board_x, board_y, board_normal):
global SMOOTHED_ORIGIN, SMOOTHED_X, SMOOTHED_Y, SMOOTHED_NORMAL
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Не надо так делать, лучше переделать в класс

Comment thread get_board_data/get_board_data_v2.py Outdated
if right_bottom in positions and right_top in positions:
heights.append(np.linalg.norm(positions[right_bottom] - positions[right_top]))

width = sum(widths) / len(widths) if len(widths) > 0 else 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

А зачем эта проверка вообще, если потенциально в width или height запишется 0? Если это произошло, то лучше вообще ничего не возвращать из функции или кидать ошибку

rvec_robot, robot_pos = DetectRobot(frame, camera_matrix, dist_coeffs, detector, robot)
if robot_pos is not None:
robot_x, robot_y, robot_theta = GetRobotState(robot_pos, rvec_robot, board_origin, board_x, board_y)
print(f"Координаты робота: x={robot_x:.4f} м, y={robot_y:.4f} м, theta={robot_theta:.4f}")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Хотелось бы визуализацию которая показывает текущий кадр и рисует всю информацию (маркеры, робота, стрелку ориентации, координаты (текстом) и т.п.)

Comment thread get_board_data/фото.jpg Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Фотографии и видео под гит не кладем

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