Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 108 additions & 13 deletions metadata_manager/ap_src_meta_fetcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,24 @@
import logging
import ap_git
import os

import re

class BoardMetadata:
Copy link
Member

Choose a reason for hiding this comment

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

Let us just call it board.

def __init__(self, id: str, name: str, attributes: dict):
self.id = id
self.name = name
self.attributes = attributes
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.attributes = attributes
self.attributes = dict(attributes)

Let us make sure the board is not affected even it the passed attributes get mutated externally.


def to_dict(self) -> dict:
# keep top-level has_can for backward compatibility
out = {
"id": self.id,
"name": self.name,
"attributes": self.attributes,
}
if "has_can" in self.attributes:
Copy link
Member

Choose a reason for hiding this comment

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

Either keep the has_can thing at top level or inside attributes property. Not both.

out["has_can"] = self.attributes["has_can"]
return out

class APSourceMetadataFetcher:
"""
Expand Down Expand Up @@ -190,8 +207,11 @@ def __get_boards_at_commit_from_cache(self,

Returns:
tuple: A tuple of two lists in order:
- A list contains boards for NON-'ap_periph' targets.
- A list contains boards for the 'ap_periph' target.
- A list of board metadata dictionaries for NON-'ap_periph' targets.
- A list of board metadata dictionaries for the 'ap_periph' target.
Each dictionary currently exposes:
- name (str): Board name.
- has_can (bool): True when the board hwdef declares CAN support.

Raises:
RuntimeError: If the method is called when caching is disabled.
Expand Down Expand Up @@ -237,8 +257,54 @@ def __exclude_boards_matching_patterns(self, boards: list, patterns: list):
ret.append(b)
return ret

def __board_has_can(self, hwdef_path: str) -> bool:
"""Return True when the hwdef file advertises CAN support."""
if not hwdef_path or not os.path.isfile(hwdef_path):
self.logger.debug(
"hwdef.dat not found while checking CAN support: %s",
hwdef_path,
)
return False

try:
with open(hwdef_path, "r", encoding="utf-8", errors="ignore") as hwdef_file:
hwdef_contents = hwdef_file.read()
except OSError as exc:
self.logger.warning(
"Failed to read hwdef.dat at %s: %s",
hwdef_path,
exc,
)
return False

combined_contents = hwdef_contents

# If the hwdef uses an include *.inc, read that file as well so
# CAN keywords defined there are detected (e.g., CubeOrange).
include_match = re.search(r"^\s*include\s+(.+\.inc)\s*$", hwdef_contents, re.MULTILINE)
if include_match:
include_name = include_match.group(1).strip()
include_path = os.path.join(os.path.dirname(hwdef_path), include_name)
if os.path.isfile(include_path):
try:
with open(include_path, "r", encoding="utf-8", errors="ignore") as inc_file:
combined_contents += "\n" + inc_file.read()
except OSError as exc:
self.logger.warning(
"Failed to read included hwdef %s: %s",
include_path,
exc,
)

return (
"CAN1" in combined_contents
or "HAL_NUM_CAN_IFACES" in combined_contents
or "CAN_P1_DRIVER" in combined_contents
or "CAN_D1_DRIVER" in combined_contents
)

def __get_boards_at_commit_from_repo(self, remote: str,
commit_ref: str) -> tuple[list, list]:
commit_ref: str) -> tuple[list[dict], list[dict]]:
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be documented in the comment.

Copy link
Author

Choose a reason for hiding this comment

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

Updated the comment

"""
Returns the tuple of boards (for both non-periph and periph targets,
in order) for a given commit from the git repo.
Expand All @@ -249,8 +315,9 @@ def __get_boards_at_commit_from_repo(self, remote: str,

Returns:
tuple: A tuple of two lists in order:
- A list contains boards for NON-'ap_periph' targets.
- A list contains boards for the 'ap_periph' target.
- A list of board metadata dictionaries for NON-'ap_periph' targets.
- A list of board metadata dictionaries for the 'ap_periph' target.
Each board dict exposes: id, name, attributes (has_can), and has_can (legacy).
Copy link
Member

Choose a reason for hiding this comment

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

I don't think there is anything like "(legacy)" in our case which we need to handle..

"""
with self.repo.get_checkout_lock():
self.repo.checkout_remote_commit_ref(
Expand All @@ -270,6 +337,8 @@ def __get_boards_at_commit_from_repo(self, remote: str,
)
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)
board_list = mod.BoardList()
hwdef_dir = getattr(board_list, "hwdef_dir", None)
non_periph_boards = mod.AUTOBUILD_BOARDS
periph_boards = mod.AP_PERIPH_BOARDS
self.logger.debug(f"non_periph_boards raw: {non_periph_boards}")
Expand All @@ -289,9 +358,33 @@ def __get_boards_at_commit_from_repo(self, remote: str,
)
self.logger.debug(f"periph_boards sorted: {periph_boards_sorted}")

def build_board_metadata(board_names: list[str]) -> list[dict]:
Copy link
Member

Choose a reason for hiding this comment

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

Please follow the pattern in the codebase. Define a separate method in this class to do this parsing. You can prefix the method names with underscores to avoid potential external usage.

board_data: list[dict] = []
for board_name in board_names:
hwdef_path = None
if hwdef_dir:
candidate_path = os.path.join(hwdef_dir, board_name, "hwdef.dat")
if os.path.isfile(candidate_path):
hwdef_path = candidate_path
else:
self.logger.debug(
"hwdef.dat not found for board %s at %s",
board_name,
candidate_path,
)

has_can = self.__board_has_can(hwdef_path) if hwdef_path else False
board = BoardMetadata(
id=board_name,
name=board_name,
attributes={"has_can": has_can},
)
board_data.append(board.to_dict())
return board_data

return (
non_periph_boards_sorted,
periph_boards_sorted,
build_board_metadata(non_periph_boards_sorted),
build_board_metadata(periph_boards_sorted),
Comment on lines +361 to +387
Copy link
Member

Choose a reason for hiding this comment

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

The repo needs to stay locked till you are done doing everything.

)

def __get_build_options_at_commit_from_repo(self,
Expand Down Expand Up @@ -334,7 +427,7 @@ def __get_build_options_at_commit_from_repo(self,
return build_options

def __get_boards_at_commit(self, remote: str,
commit_ref: str) -> tuple[list, list]:
commit_ref: str) -> tuple[list[dict], list[dict]]:
"""
Retrieves lists of boards available for building at a
specified commit for both NON-'ap_periph' and ap_periph targets
Expand All @@ -350,8 +443,8 @@ def __get_boards_at_commit(self, remote: str,

Returns:
tuple: A tuple of two lists in order:
- A list contains boards for NON-'ap_periph' targets.
- A list contains boards for the 'ap_periph' target.
- A list of board metadata dictionaries for NON-'ap_periph' targets.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of these dictionaries. We should just return a list of Board objects. We would also need to handle the caching of those.

- A list of board metadata dictionaries for the 'ap_periph' target.
"""
tstart = time.time()
if not self.caching_enabled:
Expand All @@ -376,7 +469,8 @@ def __get_boards_at_commit(self, remote: str,

if cached_boards:
boards = cached_boards
else:

if not cached_boards or boards is None:
self.logger.debug(
"Cache miss. Fetching boards from repo for "
f"commit {commid_id}."
Expand Down Expand Up @@ -407,7 +501,8 @@ def get_boards(self, remote: str, commit_ref: str,
vehicle_id (str): The vehicle ID to get the boards list for.

Returns:
list: A list of boards.
list: A list of board metadata dictionaries, each containing
the board name and whether it supports CAN (has_can).
Comment on lines +504 to +505
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
list: A list of board metadata dictionaries, each containing
the board name and whether it supports CAN (has_can).
list: A list of Boards.

"""
non_periph_boards, periph_boards = self.__get_boards_at_commit(
remote=remote,
Expand Down
7 changes: 5 additions & 2 deletions web/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ def generate():
commit_ref=commit_ref,
vehicle_id=vehicle,
)
if board not in boards_at_commit:
board_names_at_commit = [b["name"] for b in boards_at_commit]
Copy link
Member

Choose a reason for hiding this comment

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

Better define a class for board with id, name and attributes, in the attributed put the has_can thing.

Copy link
Author

Choose a reason for hiding this comment

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

Defined a BoardMetadata class for above listed

if board not in board_names_at_commit:
raise Exception("bad board")

all_features = ap_src_metadata_fetcher.get_build_options_at_commit(
Expand Down Expand Up @@ -300,9 +301,11 @@ def boards_and_features(vehicle_id, version_id):
'options' : category_options,
})
# creating result dictionary
default_board = boards[0]["name"] if boards else None

result = {
'boards' : boards,
'default_board' : boards[0],
'default_board' : default_board,
'features' : features,
}
# return jsonified result dict
Expand Down
112 changes: 90 additions & 22 deletions web/static/js/add_build.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ const Features = (() => {
var init_categories_expanded = false;

var pending_update_calls = 0; // to keep track of unresolved Promises
var currentBoards = [];
var currentFeatures = [];

function init() {
fetchVehicles();
Expand Down Expand Up @@ -408,15 +410,20 @@ function onVersionChange(new_version) {
}

function updateBoards(all_boards, new_board) {
currentBoards = all_boards || [];
let board_element = document.getElementById('board');
let old_board = board_element ? board.value : '';
let old_board = board_element ? board_element.value : '';
fillBoards(all_boards, new_board);
if (old_board != new_board) {
onBoardChange(new_board);
}
}

function onBoardChange(new_board) {
// Re-render build options to show/hide CAN based on the new board
if (currentFeatures && currentFeatures.length > 0) {
fillBuildOptions(currentFeatures);
}
fetchAndUpdateDefaults();
}

Expand Down Expand Up @@ -455,10 +462,16 @@ function fillBoards(boards, default_board) {
'<select name="board" id="board" class="form-select" aria-label="Select Board" onchange="onBoardChange(this.value);"></select>';
let boardList = document.getElementById("board")
boards.forEach(board => {
const boardName = (typeof board === 'object' && board !== null) ? board.name : board;
const hasCan = (typeof board === 'object' && board !== null) ? Boolean(board.has_can) : false;
if (!boardName) {
return;
}
let opt = document.createElement('option');
opt.value = board;
opt.innerHTML = board;
opt.selected = (board === default_board);
opt.value = boardName;
opt.innerHTML = boardName;
opt.selected = (boardName === default_board);
opt.setAttribute('data-has-can', hasCan);
boardList.appendChild(opt);
});
}
Expand Down Expand Up @@ -487,6 +500,38 @@ var toggle_all_categories = (() => {
return toggle_method;
})();

function getSelectedBoardMetadata() {
const boardSelect = document.getElementById('board');
if (!boardSelect || boardSelect.selectedIndex < 0) {
return null;
}

const selectedName = boardSelect.value;
if (!selectedName) {
return null;
}

// Prefer the cached board objects which include has_can
const fromCache = (currentBoards || []).find((board) => {
if (board && typeof board === 'object') {
return board.name === selectedName;
}
return board === selectedName;
});

if (fromCache && typeof fromCache === 'object') {
return fromCache;
}

// Fallback: derive from the selected option's data attribute
const selectedOption = boardSelect.options[boardSelect.selectedIndex];
const hasCanAttr = selectedOption ? selectedOption.getAttribute('data-has-can') : null;
return {
name: selectedName,
has_can: hasCanAttr === null ? undefined : hasCanAttr === 'true',
};
}

function createCategoryCard(category_name, options, expanded) {
options_html = "";
options.forEach(option => {
Expand Down Expand Up @@ -535,27 +580,50 @@ function createCategoryCard(category_name, options, expanded) {
}

function fillBuildOptions(buildOptions) {
// Store current features for re-rendering when board changes
currentFeatures = buildOptions;

const selectedBoard = getSelectedBoardMetadata();
// Default to hiding (false) if metadata is missing to be safe
const boardHasCan = selectedBoard && selectedBoard.has_can !== undefined
? Boolean(selectedBoard.has_can)
: false;

let output = document.getElementById('build_options');
output.innerHTML = `<div class="d-flex mb-3 justify-content-between">
<div class="d-flex d-flex align-items-center">
<p class="card-text"><strong>Available features for the current selection are:</strong></p>
</div>
<button type="button" class="btn btn-outline-primary" id="exp_col_button" onclick="toggle_all_categories();"><i class="bi bi-chevron-expand me-2"></i>Expand/Collapse all categories</button>
</div>`;

buildOptions.forEach((category, cat_idx) => {
if (cat_idx % 4 == 0) {
let new_row = document.createElement('div');
new_row.setAttribute('class', 'row');
new_row.id = 'category_'+parseInt(cat_idx/4)+'_row';
output.appendChild(new_row);
<div class="d-flex d-flex align-items-center">
<p class="card-text"><strong>Available features for the current selection are:</strong></p>
</div>
<button type="button" class="btn btn-outline-primary" id="exp_col_button" onclick="toggle_all_categories();"><i class="bi bi-chevron-expand me-2"></i>Expand/Collapse all categories</button>
</div>`;
let visibleCategoryIdx = 0;

buildOptions.forEach((category) => {
// HIDE CAN CATEGORY IF BOARD HAS NO CAN
if (category.name && category.name.includes("CAN") && boardHasCan === false) {
return;
}
let col_element = document.createElement('div');
col_element.setAttribute('class', 'col-md-3 col-sm-6 mb-2');
col_element.appendChild(createCategoryCard(category['name'], category['options'], init_categories_expanded));
document.getElementById('category_'+parseInt(cat_idx/4)+'_row').appendChild(col_element);
});
}

if (visibleCategoryIdx % 4 === 0) {
let new_row = document.createElement('div');
new_row.setAttribute('class', 'row');
new_row.id = 'category_'+parseInt(visibleCategoryIdx/4)+'_row';
output.appendChild(new_row);
}

let col_element = document.createElement('div');
col_element.setAttribute('class', 'col-md-3 col-sm-6 mb-2');
col_element.appendChild(createCategoryCard(category['name'], category['options'], init_categories_expanded));

let row = document.getElementById('category_'+parseInt(visibleCategoryIdx/4)+'_row');
if (row) {
row.appendChild(col_element);
}

visibleCategoryIdx += 1;
});
}
Comment on lines +583 to +626
Copy link
Member

Choose a reason for hiding this comment

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

Instead of hiding stuff at the frontend, we should make sure they don't even get returned from the backend. The filtering logic has to go in the backend.

Also, when we make some features disappear the features which are dependent on those should also disappear.


// returns a Promise
// the promise is resolved when we recieve status code 200 from the AJAX request
Expand Down