Skip to content

[RFC] Armbianmonitor refactor#9327

Open
EvilOlaf wants to merge 7 commits intoarmbian:mainfrom
EvilOlaf:armbianmonitor-refactor
Open

[RFC] Armbianmonitor refactor#9327
EvilOlaf wants to merge 7 commits intoarmbian:mainfrom
EvilOlaf:armbianmonitor-refactor

Conversation

@EvilOlaf
Copy link
Member

@EvilOlaf EvilOlaf commented Feb 1, 2026

Description

  • split log upload into dedicated tool

    • adjust its logic to auto upload but allow user to output to stdout by simple key press
    • leave -u/-U in armbianmonitor intact but add deprecation notice
  • armbianmonitor

    • remove tons of dead code
    • remove old benchmark tools
    • remove rpimonitor and dirty binary patch hack
    • remove references to EOL userspaces and 3.y kernel from middle-age
      • perhaps add references to sbcbench later for proper results
    • remove old debug mode
      • was referencing to ix.io which has been dead for years and nobody ever complained
    • various fixes

@rpardini you may wanna have a look at this...or not :D

How Has This Been Tested?

  • only armbian-debug has been tested so far

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

  • New Features

    • Added a dedicated debug utility to collect extensive system diagnostics, with multiple upload endpoints, a 5s interactive countdown, optional stdout fallback (line-numbered output), sanitization of sensitive-looking data, and automatic uploader checks.
  • Refactor

    • Simplified the existing monitor tool by removing legacy support-upload flows and deprecating upload flags; help text and options updated to redirect debug/upload actions to the new utility.

Perhaps add a reference to sbcbench later for proper benchmarking
-D was referencing to ix.io which has been dead for years now and nobody complained
7z benchmark was interesting years ago when sbcs weren't as powerful as they are today. Perhaps also here reference to sbcbench for proper results.
@EvilOlaf EvilOlaf requested a review from a team as a code owner February 1, 2026 13:15
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 1, 2026

📝 Walkthrough

Walkthrough

Adds a new armbian-debug utility that collects and uploads comprehensive Armbian debug data (with interactive stdout fallback) and trims/deprecates upload-related functionality from armbianmonitor, redirecting support-info upload flows to armbian-debug.

Changes

Cohort / File(s) Summary
New debug utility
packages/bsp/common/usr/bin/armbian-debug
Adds a new Bash script providing CollectSupportInfo() and Main(); collects extensive system diagnostics, sanitizes data, attempts uploads to multiple paste servers via curl (tracks PIPESTATUS), and falls back to interactive/stdout dump.
armbianmonitor cleanup
packages/bsp/common/usr/bin/armbianmonitor
Removes legacy upload/collector functions (InstallRPiMonitor, PatchRPiMonitor_for_sun8i, CollectSupportInfo, Run7ZipBenchmark, PreRequisits) and deprecates -u/-U upload flags, updating ParseOptions() getopts string and help text to redirect to armbian-debug.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant armbian-debug
    participant curl
    participant PasteServer
    participant ShellCmds

    User->>armbian-debug: run script (interactive)
    armbian-debug->>User: 5s prompt to switch to stdout
    armbian-debug->>ShellCmds: CollectSupportInfo() (dmesg, hw, lshw, etc.)
    ShellCmds-->>armbian-debug: diagnostic payload
    armbian-debug->>armbian-debug: sanitize payload (redact IPs)
    armbian-debug->>curl: upload to PasteServer A
    curl-->>armbian-debug: exit status (PIPESTATUS)
    alt success
        PasteServer-->>armbian-debug: URL
        armbian-debug->>User: success message + URL
    else failure
        armbian-debug->>curl: try PasteServer B...
        alt all attempts fail
            armbian-debug->>User: fallback -> print numbered, redacted stdout
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

"I dug through logs and kernel rhyme,
I hopped through dmesg, one byte at a time.
If uploads fall and paste won't stick,
I'll print the lines — quick, quick, quick! 🐇
A little debug, a little cheer, Armbian tales now crystal clear."

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title '[RFC] Armbianmonitor refactor' is vague and generic—it uses a broad term ('refactor') without clearly conveying the main change (splitting debug upload into dedicated tool). Consider a more specific title like '[RFC] Extract debug upload to armbian-debug tool' or 'Split armbianmonitor: move debug upload to dedicated armbian-debug script' to clearly convey the primary intent.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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.

@github-actions github-actions bot added 02 Milestone: First quarter release size/large PR with 250 lines or more Needs review Seeking for review BSP Board Support Packages labels Feb 1, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/bsp/common/usr/bin/armbian-debug`:
- Around line 57-106: The curl call inside the paste_server upload loop can hang
indefinitely; update the curl invocation in the for paste_server loop (the
pipeline that ends with curl -s --fail --data-binary `@-`
"https://${paste_server}/log") to include connection and overall timeouts (e.g.
--connect-timeout 10 and --max-time 30) so stalled servers fail fast and allow
the script to fall back to stdout; keep the rest of the pipeline and PIPESTATUS
handling unchanged.

Comment on lines 57 to 106
# Collect and process the info
if [ "$force_stdout" = true ]; then
# Output to stdout with line numbers
CollectSupportInfo |
sed -E 's/([0-9]{1,3}\.)([0-9]{1,3}\.)([0-9]{1,3}\.)([0-9]{1,3})/XXX.XXX.\3\4/g' |
awk '!NF{$0=" "}1' | nl -
echo -e "\nPlease upload the ${BOLD}whole output${NC} above to an online pasteboard service\nand provide the URL in the forum where you have been asked for this.\n"
else
# Try to upload to paste servers
upload_success=false

for paste_server in "${paste_servers[@]}"; do
echo "Collecting info and sending to ${paste_server}..."
declare -i counter=0
{
LC_ALL=C date
echo "-----------------------------------------------------------------------------------------------------------------------------"
dmesg --color=always
echo "-----------------------------------------------------------------------------------------------------------------------------"
CollectSupportInfo || echo "Error collecting support info"
} |
sed -E 's/([0-9]{1,3}\.)([0-9]{1,3}\.)([0-9]{1,3}\.)([0-9]{1,3})/XXX.XXX.\3\4/g' |
curl -s --fail --data-binary @- "https://${paste_server}/log"

# Save PIPESTATUS before any other command
pipe_status=("${PIPESTATUS[@]}")

# Check PIPESTATUS
for i in "${pipe_status[@]}"; do
counter=$((counter + 1))
if [[ $i -ne 0 ]]; then
echo "Failed (pipe ${counter} returned ${i})."
continue 2
fi
done

# If we get here, upload succeeded
upload_success=true
break
done

if [ "$upload_success" = true ]; then
echo -e "\n${BOLD}Upload successful!${NC} Please post the URL at the place where you have been asked for.\n"
else
echo -e "\n${BOLD}All upload attempts failed. Outputting to stdout instead:${NC}\n"
CollectSupportInfo |
sed -E 's/([0-9]{1,3}\.)([0-9]{1,3}\.)([0-9]{1,3}\.)([0-9]{1,3})/XXX.XXX.\3\4/g' |
awk '!NF{$0=" "}1' | nl -
echo -e "\nPlease upload the ${BOLD}whole output${NC} above to an online pasteboard service\nand provide the URL in the forum where you have been asked for this.\n"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

curl --help all 2>&1 | grep -E -- '--connect-timeout|--max-time'

Repository: armbian/build

Length of output: 207


Add curl timeouts to prevent indefinite hangs on stalled paste servers.

Without connect/transfer timeouts, curl can block indefinitely if a paste server becomes unresponsive, preventing the script from reaching the fallback stdout output. Use --connect-timeout 10 and --max-time 30 to ensure the script remains responsive.

Proposed fix
-				curl -s --fail --data-binary `@-` "https://${paste_server}/log"
+				curl -s --fail --connect-timeout 10 --max-time 30 --data-binary `@-` "https://${paste_server}/log"
🤖 Prompt for AI Agents
In `@packages/bsp/common/usr/bin/armbian-debug` around lines 57 - 106, The curl
call inside the paste_server upload loop can hang indefinitely; update the curl
invocation in the for paste_server loop (the pipeline that ends with curl -s
--fail --data-binary `@-` "https://${paste_server}/log") to include connection and
overall timeouts (e.g. --connect-timeout 10 and --max-time 30) so stalled
servers fail fast and allow the script to fall back to stdout; keep the rest of
the pipeline and PIPESTATUS handling unchanged.

@EvilOlaf
Copy link
Member Author

EvilOlaf commented Feb 3, 2026

I have no idea where this conflict comes from and how I produced that but nevertheless this is more POC and RFC, so....

@igorpecovnik igorpecovnik added 05 Milestone: Second quarter release and removed 02 Milestone: First quarter release labels Feb 3, 2026
@rpardini
Copy link
Member

rpardini commented Feb 4, 2026

I have very strict orders from my psychiatrist to never look at this again 😉 that said I fully trust you on this

@EvilOlaf
Copy link
Member Author

EvilOlaf commented Feb 4, 2026

Haha.
Well fair warning: I did not touch the internals as they work...much. Most is cleanup and remove old stuff and split functionality.

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

Labels

05 Milestone: Second quarter release BSP Board Support Packages Needs review Seeking for review size/large PR with 250 lines or more

Development

Successfully merging this pull request may close these issues.

3 participants