Skip to content

Conversation

puhitaku
Copy link
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
Contributor Author

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

@ishuah
Copy link
Owner

ishuah commented Feb 10, 2025

Thank you for this improvement PR @puhitaku

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

@@ -157,7 +157,7 @@ func main() {
case Esc:
connect.Write([]byte{'\x1b'})
case CtrlBackslash:
fmt.Print("bye!")
Copy link
Owner

Choose a reason for hiding this comment

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

Nice touch 🤌

for {
response, err := c.portReader.ReadBytes('\n')
Copy link
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
Owner

ishuah commented Feb 10, 2025

Approved and merged. Thank you!

@ishuah
Copy link
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