Skip to content

Conversation

@akalipetis
Copy link
Member

No description provided.

@akalipetis akalipetis marked this pull request as ready for review June 11, 2025 13:17
@akalipetis
Copy link
Member Author

@tsangiotis the update is looking good!

@akalipetis
Copy link
Member Author

@tsangiotis if you have more changes coming, let me know when you're done to give this another look (if you want to).

@tsangiotis
Copy link

For now I think we are ok. Any changes can happen down the road.

Copilot AI review requested due to automatic review settings October 9, 2025 16:36
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 comprehensive MaintNode building stage (stage2arp) to the Raspberry Pi image generation pipeline. The stage creates a custom image with specialized hardware support, networking capabilities, and remote management tools for IoT/maintenance applications.

Key changes:

  • Introduces stage2arp with 9 sub-stages for complete MaintNode provisioning
  • Adds Python environment setup with Poetry for dependency management
  • Implements web-based configuration interface with hotspot capabilities for field deployment
  • Integrates industrial hardware support (DAQHATs, ULDAQ) and remote access tools (Tailscale, FRP)

Reviewed Changes

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

Show a summary per file
File Description
stage2arp/* Complete MaintNode stage with Python setup, source retrieval, web config, hardware drivers, Node-RED, Tailscale, and FRP client
stage1/00-boot-files/files/config.txt Enables I2C and SPI interfaces for hardware communication
stage2/03-accept-mathematica-eula/00-debconf Removes Wolfram engine EULA acceptance (not needed for MaintNode)
build.sh Updates default settings for Greek locale, SSH enabled, and adds Tailscale auth key support
Makefile Adds build automation with Docker SSH forwarding and environment variable support
Dockerfile Adds SSH client and Python tools for build environment
Comments suppressed due to low confidence (1)

stage2arp/04-maintnode/files/maintnode-setup.sh:1

  • Comment states 'Hold for 10 seconds to enable' but the hold_time parameter is set to 8 seconds. The comment should be corrected to match the actual hold time.
#!/bin/bash -e

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

Comment on lines +14 to +17
envsubst '$FIRST_USER_NAME' < files/frpc.ini > "${ROOTFS_DIR}/usr/local/etc/frp/frpc.ini"

# Install frpc systemd service file with variable substitution
envsubst '$FIRST_USER_NAME' < files/frpc.service > "${ROOTFS_DIR}/lib/systemd/system/frpc.service"
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

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

The envsubst commands use '$FIRST_USER_NAME' but the actual template files (frpc.ini and frpc.service) contain '$MAINTNODE_ID' variables, not '$FIRST_USER_NAME'. This will result in incomplete variable substitution.

Suggested change
envsubst '$FIRST_USER_NAME' < files/frpc.ini > "${ROOTFS_DIR}/usr/local/etc/frp/frpc.ini"
# Install frpc systemd service file with variable substitution
envsubst '$FIRST_USER_NAME' < files/frpc.service > "${ROOTFS_DIR}/lib/systemd/system/frpc.service"
envsubst '$MAINTNODE_ID' < files/frpc.ini > "${ROOTFS_DIR}/usr/local/etc/frp/frpc.ini"
# Install frpc systemd service file with variable substitution
envsubst '$MAINTNODE_ID' < files/frpc.service > "${ROOTFS_DIR}/lib/systemd/system/frpc.service"

Copilot uses AI. Check for mistakes.
Comment on lines +6 to +7
install -m 644 files/tailscale-up.service "${ROOTFS_DIR}/etc/systemd/system/tailscale-up.service"
ls ${ROOTFS_DIR}/etc/systemd/system/ No newline at end of file
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

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

Debug command 'ls' should be removed from production code as it serves no functional purpose and clutters the output.

Suggested change
install -m 644 files/tailscale-up.service "${ROOTFS_DIR}/etc/systemd/system/tailscale-up.service"
ls ${ROOTFS_DIR}/etc/systemd/system/
install -m 644 files/tailscale-up.service "${ROOTFS_DIR}/etc/systemd/system/tailscale-up.service"

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,6 @@
#!/bin/bash -e

ls -la /tmp ${ROOTFS_DIR}/tmp
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

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

Debug command 'ls -la' should be removed from production code as it serves no functional purpose and clutters the output.

Suggested change
ls -la /tmp ${ROOTFS_DIR}/tmp

Copilot uses AI. Check for mistakes.
on_chroot << EOF
cd /tmp
tar -xzf maintnode-agent.tar.gz
ls -lR
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

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

Debug command 'ls -lR' should be removed from production code as it serves no functional purpose and clutters the output.

Suggested change
ls -lR

Copilot uses AI. Check for mistakes.
from signal import pause

# GPIO 17 is active low
hotspot_button = Button(17, pull_up=True, hold_time=8) # Hold for 10 seconds to enable
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

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

Comment states 'Hold for 10 seconds to enable' but the hold_time parameter is set to 8 seconds. The comment should be corrected to match the actual hold time.

Copilot uses AI. Check for mistakes.
def handle_button_release():
global hotspot_enabled
if not hotspot_enabled:
# Enable hotspot when button is held for 10 seconds and released
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

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

Comment states button must be held for '10 seconds' but the actual hold_time is configured as 8 seconds on line 6. Comment should be updated to reflect the correct duration.

Copilot uses AI. Check for mistakes.
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.

3 participants