Skip to content

Conversation

@QwaSeeK
Copy link
Member

@QwaSeeK QwaSeeK commented Oct 29, 2025

Description

Add post-build Armbian extension for burnable JetHub boards (for j80, j100 and j200)

How Has This Been Tested?

Builds and flashes ok

@github-actions github-actions bot added size/medium PR with more then 50 and less then 250 lines 11 Milestone: Fourth quarter release Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... Framework Framework components labels Oct 29, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

DEFAULT_CONSOLE="serial"
HAS_VIDEO_OUTPUT="no"
MODULES_BLACKLIST="rtw88_8822cs"
enable_extension "jethub-burn"
Copy link
Member

Choose a reason for hiding this comment

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

blank line at end of file

FULL_DESKTOP="yes"
SERIALCON="ttyAML0"
#BOOT_LOGO="desktop"
enable_extension "jethub-burn"
Copy link
Member

Choose a reason for hiding this comment

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

blank line at end of file

DEFAULT_CONSOLE="serial"
HAS_VIDEO_OUTPUT="no"
MODULES_BLACKLIST="rtw88_8822cs"
enable_extension "jethub-burn"
Copy link
Member

Choose a reason for hiding this comment

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

blank line at end of file

@@ -0,0 +1,117 @@
#!/usr/bin/env bash
# Extension: jethub-burn
Copy link
Member

Choose a reason for hiding this comment

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

insert JetHome copyright

#
# SPDX-License-Identifier: GPL-2.0
# Copyright (c) 2025 JetHome
# This file is a part of the Armbian Build Framework https://github.com/armbian/build/
#

fetch_from_repo "${repo_url}" "${name}" "${ref}"

local packer_path
packer_path="$(find "${base}" -maxdepth 12 -type f -path '*/tools/aml_image_v2_packer_new' -print -quit)"
Copy link
Member

@adeepn adeepn Oct 30, 2025

Choose a reason for hiding this comment

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

you cat set tool path to ${SRC}/cache/sources/jethome-tools/tools/aml_image_v2_packer_new, so do not use find

packer_path="$(find "${base}" -maxdepth 12 -type f -path '*/tools/aml_image_v2_packer_new' -print -quit)"
[[ -n "${packer_path}" ]] || exit_with_error "aml_image_v2_packer_new not found under ${base}"

declare -g TOOLS_DIR="$(dirname "$(dirname "${packer_path}")")"
Copy link
Member

Choose a reason for hiding this comment

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

and there use ${SRC}/cache/sources/ as base path for fethed artifacts


display_alert "make_burn" "Extracting partitions (losetup -P)..." "info"
local loopdev
loopdev="$(losetup --find --show -P "$input_img")" || exit_with_error "losetup failed for $input_img"
Copy link
Member

Choose a reason for hiding this comment

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

find base partition file, do not re-extract builded image
or
use gdisk/dd magic for copy partition without use loop device.

@adeepn adeepn requested a review from Copilot October 30, 2025 11:06
@adeepn adeepn self-assigned this Oct 30, 2025
Copy link

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 adds a new extension jethub-burn that automatically converts Armbian .img files into Amlogic burn image format after building, specifically for JetHub boards (j80, j100, j200).

Key changes:

  • New extension script implementing Amlogic burn image conversion tooling
  • Enables the extension for three JetHub board configurations

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
extensions/jethub-burn.sh Implements bootstrap tools fetching, DTB building, partition extraction, and burn image packing logic
config/boards/jethubj80.conf Enables jethub-burn extension for JetHub J80 board
config/boards/jethubj100.conf Enables jethub-burn extension for JetHub J100 board
config/boards/jethubj200.conf Enables jethub-burn extension for JetHub J200 board

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

Comment on lines +59 to +61
local loopdev
loopdev="$(losetup --find --show -P "$input_img")" || exit_with_error "losetup failed for $input_img"
trap 'losetup -d "$loopdev" 2>/dev/null || true; rm -rf "$tmpdir"' RETURN
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

The trap command on line 61 overrides the existing trap on line 39, which means the tmpdir cleanup from line 39 won't execute if the losetup succeeds. This could lead to tmpdir not being cleaned up properly. The trap should combine both cleanup actions instead of replacing the first one.

Copilot uses AI. Check for mistakes.
Comment on lines +93 to +98
jethubj80) dts_name="meson-gxl-s905w-jethome-jethub-j80.dts"; bins_subdir="j80" ;;
jethubj100) dts_name="meson-axg-jethome-jethub-j100.dts"; bins_subdir="j100" ;;
jethubj200) dts_name="meson-sm1-jethome-jethub-j200.dts"; bins_subdir="j200" ;;
*) exit_with_error "Unsupported board: ${BOARD} (supported: j80, j100, j200)";;
esac

Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

[nitpick] The board-to-configuration mapping is hardcoded with duplicate information. The bins_subdir could be derived from the board name (e.g., 'jethubj80' → 'j80') to reduce duplication. Only the dts_name needs to be board-specific.

Suggested change
jethubj80) dts_name="meson-gxl-s905w-jethome-jethub-j80.dts"; bins_subdir="j80" ;;
jethubj100) dts_name="meson-axg-jethome-jethub-j100.dts"; bins_subdir="j100" ;;
jethubj200) dts_name="meson-sm1-jethome-jethub-j200.dts"; bins_subdir="j200" ;;
*) exit_with_error "Unsupported board: ${BOARD} (supported: j80, j100, j200)";;
esac
jethubj80) dts_name="meson-gxl-s905w-jethome-jethub-j80.dts"; ;;
jethubj100) dts_name="meson-axg-jethome-jethub-j100.dts"; ;;
jethubj200) dts_name="meson-sm1-jethome-jethub-j200.dts"; ;;
*) exit_with_error "Unsupported board: ${BOARD} (supported: j80, j100, j200)";;
esac
bins_subdir="${BOARD#jethub}" # Extract substring after 'jethub'

Copilot uses AI. Check for mistakes.
@igorpecovnik igorpecovnik added the Work in progress Unfinished / work in progress label Nov 2, 2025
@rpardini
Copy link
Member

rpardini commented Nov 8, 2025

Also: please don't use trap directly, as that's the source of much woe. Armbian has a "trap-framework" at lib/functions/logging/traps.sh -- if needed you could use add_cleanup_handler() instead which would play nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

11 Milestone: Fourth quarter release Framework Framework components Hardware Hardware related like kernel, U-Boot, ... Needs review Seeking for review size/medium PR with more then 50 and less then 250 lines Work in progress Unfinished / work in progress

Development

Successfully merging this pull request may close these issues.

4 participants