refactor: update serial port handling and dependencies #1

2025-08-06T21:14:35Z

jonasclaes commented

2025-08-06 21:14:35 +00:00

(Migrated from github.com)

No description provided.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2025-08-07 12:38:39 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR refactors the serial port handling by migrating from the jacobsa/go-serial library to go.bug.st/serial and modernizes the printer communication architecture.

Replaces serial library dependency for improved compatibility and maintenance
Simplifies printer communication by removing buffering layers and direct serial operations
Updates receiver variable naming for better consistency across the codebase

Reviewed Changes

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

File	Description
pkg/service/print-service.go	Updates serial port initialization, removes direct port read operations, and standardizes receiver naming
pkg/escpos/escpos.go	Removes buffered writing, adds serial-specific buffer reset functionality, and updates receiver naming
go.mod	Updates dependency from jacobsa/go-serial to go.bug.st/serial
cmd/esc-test/main.go	Removes explicit Print() call from test code

## Pull Request Overview This PR refactors the serial port handling by migrating from the `jacobsa/go-serial` library to `go.bug.st/serial` and modernizes the printer communication architecture. - Replaces serial library dependency for improved compatibility and maintenance - Simplifies printer communication by removing buffering layers and direct serial operations - Updates receiver variable naming for better consistency across the codebase ### Reviewed Changes Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments. | File | Description | | ---- | ----------- | | pkg/service/print-service.go | Updates serial port initialization, removes direct port read operations, and standardizes receiver naming | | pkg/escpos/escpos.go | Removes buffered writing, adds serial-specific buffer reset functionality, and updates receiver naming | | go.mod | Updates dependency from jacobsa/go-serial to go.bug.st/serial | | cmd/esc-test/main.go | Removes explicit Print() call from test code |

pkg/escpos/escpos.go

					
				@ -1,95 +1,99 @@

				package escpos

Using log.Fatalf will terminate the entire application when buffer reset fails. This should return an error or use a less severe logging level to allow graceful error handling.

pkg/service/print-service.go

					
				@ -27,24 +26,48 @@ type StatusRequest struct {

				}

The serial port path is hardcoded to "/dev/ttyUSB0" which may not work across different systems. Consider making this configurable through environment variables or configuration parameters.

	serialPortPath := os.Getenv("SERIAL_PORT_PATH")
	if serialPortPath == "" {
		serialPortPath = "/dev/ttyUSB0"
	}

	port, err := serial.Open(serialPortPath, mode)

The serial port path is hardcoded to "/dev/ttyUSB0" which may not work across different systems. Consider making this configurable through environment variables or configuration parameters. ```suggestion serialPortPath := os.Getenv("SERIAL_PORT_PATH") if serialPortPath == "" { serialPortPath = "/dev/ttyUSB0" } port, err := serial.Open(serialPortPath, mode) ```

pkg/service/print-service.go

					
				@ -147,7 +164,7 @@ func (pm *PrintService) Print(ctx context.Context, data []byte) error {

					}

					select {

The print method now writes to ps.printer instead of the serial port directly, but the printer should write to the underlying serial port. This change may break the printing functionality.

	_, err := ps.port.Write(data)
	if err != nil {
		return fmt.Errorf("failed to write to serial port: %w", err)

The print method now writes to ps.printer instead of the serial port directly, but the printer should write to the underlying serial port. This change may break the printing functionality. ```suggestion _, err := ps.port.Write(data) if err != nil { return fmt.Errorf("failed to write to serial port: %w", err) ```

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2025-08-07 14:02:05 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR modernizes the serial port handling by replacing the deprecated github.com/jacobsa/go-serial library with go.bug.st/serial and introduces a configuration system using TOML files. The changes make the application more configurable and use a more maintained serial library.

Replaces deprecated serial library with modern alternative
Adds comprehensive TOML-based configuration system with environment variable support
Updates ESC/POS implementation to work with new serial interface

Reviewed Changes

Copilot reviewed 10 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/service/print-service.go	Updated to use new serial library and accept configuration service dependency
pkg/service/config-service.go	New service for loading and managing TOML configuration with default values
pkg/model/app-config.go	Defines configuration structure for server and printer settings
pkg/escpos/escpos.go	Refactored to work with new serial interface and improved error handling
pkg/bootstrap/services-bootstrap.go	Updated to initialize config service and pass it to print service
pkg/bootstrap/bootstrap.go	Uses configuration for server address instead of hardcoded values
go.mod	Updated dependencies to include new serial library and TOML parser
config.example.toml	Example configuration file showing available options
cmd/esc-test/main.go	Removed call to deprecated Print() method
README.md	Added comprehensive documentation for configuration system

## Pull Request Overview This PR modernizes the serial port handling by replacing the deprecated `github.com/jacobsa/go-serial` library with `go.bug.st/serial` and introduces a configuration system using TOML files. The changes make the application more configurable and use a more maintained serial library. - Replaces deprecated serial library with modern alternative - Adds comprehensive TOML-based configuration system with environment variable support - Updates ESC/POS implementation to work with new serial interface ### Reviewed Changes Copilot reviewed 10 out of 13 changed files in this pull request and generated 3 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | pkg/service/print-service.go | Updated to use new serial library and accept configuration service dependency | | pkg/service/config-service.go | New service for loading and managing TOML configuration with default values | | pkg/model/app-config.go | Defines configuration structure for server and printer settings | | pkg/escpos/escpos.go | Refactored to work with new serial interface and improved error handling | | pkg/bootstrap/services-bootstrap.go | Updated to initialize config service and pass it to print service | | pkg/bootstrap/bootstrap.go | Uses configuration for server address instead of hardcoded values | | go.mod | Updated dependencies to include new serial library and TOML parser | | config.example.toml | Example configuration file showing available options | | cmd/esc-test/main.go | Removed call to deprecated Print() method | | README.md | Added comprehensive documentation for configuration system | </details>

pkg/service/print-service.go

					
				@ -27,24 +26,48 @@ type StatusRequest struct {

				}

The StopBits field is set to OneStopBit initially but then immediately overridden by the switch statement below. Consider removing this redundant initialization to improve code clarity.

The StopBits field is set to OneStopBit initially but then immediately overridden by the switch statement below. Consider removing this redundant initialization to improve code clarity. ```suggestion ```

The Parity field is set to NoParity initially but then immediately overridden by the switch statement below. Consider removing this redundant initialization to improve code clarity.

The Parity field is set to NoParity initially but then immediately overridden by the switch statement below. Consider removing this redundant initialization to improve code clarity. ```suggestion ```

pkg/service/print-service.go

					
				@ -172,3 +188,4 @@

					case ps.statusQueue <- req:

						// Status request queued successfully, wait for response

						select {

						case status := <-response:

The print method is writing data to ps.printer instead of ps.port. This appears inconsistent with the previous implementation which wrote directly to the port. Verify that the ESCPOS Write method handles the data correctly for printing.

	_, err := ps.port.Write(data)

The print method is writing data to ps.printer instead of ps.port. This appears inconsistent with the previous implementation which wrote directly to the port. Verify that the ESCPOS Write method handles the data correctly for printing. ```suggestion _, err := ps.port.Write(data) ```

Sign in to join this conversation.

No reviewers