Skip to content

Unbuffer reading the port etc.#21

Merged
ishuah merged 4 commits intoishuah:masterfrom
puhitaku:puhitaku/unbuffer
Feb 10, 2025
Merged

Unbuffer reading the port etc.#21
ishuah merged 4 commits intoishuah:masterfrom
puhitaku:puhitaku/unbuffer

Conversation

@puhitaku
Copy link
Copy Markdown
Contributor

Hi, thanks for the nice tool!

I've found some things to improve for better bifrost, so I've made some commits here:

5fd6351 Run go mod tidy

  • Go compiler asked me to do so because there were some packages like "xgo" that aren't in use anymore.

e970ec5 Unbuffer reading the port for realtime interaction

  • In the original code, bufio was used to buffer the line and read the RX line-by-line.
  • Since there is a buffer on the kernel side, we won't need additional buffering.
  • Additionally, for an interactive session like connecting the UART on a Linux machine's serial tty, any output incl. echo back will be transmitted without newline. With buffering a line, I couldn't observe any output on display until I press Enter.
  • I used io.ReadFull with a single byte buffer instead, so now bifrost can receive and display every single byte as soon as it's received.

dd03a78 Farewell with a fresh line

  • There should be \n around "bye!" so that I can take screenshots of the log intact, and the friendly farewell will be more visible to users 😉

I hope these changes are ok for you! Thanks!

@puhitaku
Copy link
Copy Markdown
Contributor Author

Oops, I forgot to update the tests. I'll add commits soon ...

@ishuah
Copy link
Copy Markdown
Owner

ishuah commented Feb 10, 2025

Thank you for this improvement PR @puhitaku

I'm reviewing, will get back to you in a bit.

case Esc:
connect.Write([]byte{'\x1b'})
case CtrlBackslash:
fmt.Print("bye!")
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice touch 🤌

func (c *Connect) read() {
buf := make([]byte, 1)
for {
response, err := c.portReader.ReadBytes('\n')
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

A bufferless approach makes much more sense.

@ishuah ishuah merged commit ed56e98 into ishuah:master Feb 10, 2025
@ishuah
Copy link
Copy Markdown
Owner

ishuah commented Feb 10, 2025

Approved and merged. Thank you!

@ishuah
Copy link
Copy Markdown
Owner

ishuah commented Feb 10, 2025

I will create a new release this week.

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.

2 participants