demo:button demo

[luoliwoshang]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @luoliwoshang, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request adds a simple button demo to the repository. The demo configures a GPIO pin, reads the button state, and prints 'yes' or 'no' accordingly. It also includes direct memory access for hardware initialization.

Highlights

• Button Demo Implementation: This PR introduces a basic button demo using the goplus/emb/machine library. It configures a GPIO pin as an input and prints 'yes' or 'no' based on the button's state.
• Direct Memory Access: The code directly accesses memory locations using unsafe and uintptr to set specific memory addresses, likely for hardware initialization related to the button.
• Infinite Loop: The main function contains an infinite loop that continuously reads the button state and prints the corresponding output.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request adds a demonstration program for reading a button state on an embedded device. The code correctly configures a GPIO pin as input and reads its state in a loop. However, it uses hardcoded magic numbers to disable watchdog timers. I've recommended replacing these with named constants to improve readability and maintainability. This is a critical improvement for low-level hardware interaction code.

[gemini-code-assist]

The code uses magic numbers for memory addresses and values to disable watchdog timers. This makes the code hard to read, understand, and maintain. These magic numbers should be replaced with named constants that explain their purpose. This will greatly improve code clarity and make it easier to verify the register manipulation logic.

Please define these constants at the package level, for example, before the main function:

const (
	// RTC Watchdog Timer write protection
	wdtRTCProtectReg = 0x3ff4808C

	// RTC Watchdog Timer configuration
	wdtRTCConfig0Reg = 0x3ff480A4
	wdtRTCKeyValue   = 0x50D83AA1 // Key to unlock RTC WDT registers

	// Timer Group 0 Watchdog Timer write protection
	wdtT0ProtectReg = 0x3ff5f048
)

Suggested change

	StoreUint32((*uint32)(unsafe.Pointer(uintptr(0x3ff480A4))), 0x50D83AA1)
	StoreUint32((*uint32)(unsafe.Pointer(uintptr(0x3ff4808C))), 0)
	StoreUint32((*uint32)(unsafe.Pointer(uintptr(0x3ff5f048))), 0)
	StoreUint32((*uint32)(unsafe.Pointer(uintptr(wdtRTCConfig0Reg))), wdtRTCKeyValue)
	StoreUint32((*uint32)(unsafe.Pointer(uintptr(wdtRTCProtectReg))), 0)
	StoreUint32((*uint32)(unsafe.Pointer(uintptr(wdtT0ProtectReg))), 0)

[fennoai]

Critical: Undocumented magic numbers and hardware addresses

These hard-coded register addresses and magic values make the code unmaintainable and potentially unsafe:

• 0x3ff480A4 = RTC_CNTL.WDTWPROTECT register
• 0x50D83AA1 = Watchdog unlock key
• 0x3ff4808C = RTC_CNTL.WDTCONFIG0 register
• 0x3ff5f048 = TIMG0.WDTCONFIG0 register

Issues:

1. No documentation explaining these disable watchdog timers
2. Users can't understand or safely modify this code
3. The project already has proper abstractions in device/esp/esp32.go that should be used

Recommendation:

// Disable ESP32 watchdog timers to prevent automatic resets
const (
    RTC_CNTL_WDTWPROTECT_REG = 0x3ff480A4
    RTC_CNTL_WDT_WKEY = 0x50D83AA1  // Unlock key
    RTC_CNTL_WDTCONFIG0_REG = 0x3ff4808C
    TIMG0_WDTCONFIG0_REG = 0x3ff5f048
)

// Unlock and disable RTC watchdog
StoreUint32((*uint32)(unsafe.Pointer(uintptr(RTC_CNTL_WDTWPROTECT_REG))), RTC_CNTL_WDT_WKEY)
StoreUint32((*uint32)(unsafe.Pointer(uintptr(RTC_CNTL_WDTCONFIG0_REG))), 0)

// Disable Timer Group 0 watchdog
StoreUint32((*uint32)(unsafe.Pointer(uintptr(TIMG0_WDTCONFIG0_REG))), 0)

Or better yet, use the existing device package abstractions.

[fennoai]

Performance: Excessive I/O operations

The code prints to UART on every loop iteration (every second), regardless of whether the button state changed. This wastes CPU cycles and power.

Recommendation: Only print on state changes:

var lastState bool
for {
    currentState := buttonPin.Get()
    if currentState != lastState {
        if currentState {
            println("Button: HIGH")
        } else {
            println("Button: LOW")
        }
        lastState = currentState
    }
    sleep(1)
}

This reduces unnecessary I/O operations by ~99% when the button is in a stable state.

[fennoai]

Documentation: Missing package documentation

Add documentation explaining:

• Target hardware (ESP32)
• What the demo demonstrates
• Hardware setup requirements
• Why watchdog timers are disabled

Suggestion:

// This demo demonstrates reading button input on ESP32-based boards.
// It continuously polls GPIO34 for button state and prints the result.
//
// Hardware requirements:
//   - ESP32 board (tested on M5Stack)
//   - Button connected to GPIO34 (or use built-in button)
//
// Note: This demo disables watchdog timers to prevent resets during execution.
//
//go:build esp32

package main

[fennoai]

Documentation: Undocumented go:linkname directives

These advanced features need explanation for demo/educational purposes:

// StoreUint32 performs an atomic store to a memory address.
// Linked to llgo's atomic store for bare-metal operations.
//
//go:linkname StoreUint32 llgo.atomicStore
func StoreUint32(addr *uint32, val uint32)

// sleep pauses execution for n seconds using llgo runtime.
//
//go:linkname sleep sleep
func sleep(n int)

Note: The StoreUint32 functionality already exists in runtime/volatile/volatile.go and could be imported instead.

[fennoai]

Code Review Summary

This button demo adds basic GPIO input functionality but has critical documentation and code quality issues that should be addressed:

Critical Issues:

• Hard-coded register addresses (lines 16-18) lack any documentation explaining they disable watchdog timers
• Missing package-level documentation about target hardware and purpose
• Undocumented go:linkname directives

Performance:

• Excessive UART I/O operations - prints every second regardless of state changes (recommend: only print on changes)

Recommendations:

1. Add comprehensive documentation (package docs, comments for magic numbers)
2. Use named constants for hardware addresses or leverage existing device/esp package
3. Add build constraint: //go:build esp32
4. Optimize printing to only occur on button state changes

The GPIO reading logic itself is sound, but the demo needs better documentation to serve its educational purpose.