Skip to content

Conversation

@sago35
Copy link
Member

@sago35 sago35 commented Nov 4, 2020

The following approaches were used to speed up the process.

  • It used to send every 1 byte, but it now holds a maximum of 63 bytes before sending
  • if the buffer is full, send buffer
  • If there is data at the time of the handleUSBIRQ every 1ms, send buffer

Unlike #1452, it does not require additional memory.
The code is a little more complicated, but it looks like it works very well.

  • 13 byte : fmt.Printf("hello world\r\n")
    • old : 202.814 us
    • new : 51.330 us
  • 11 byte : println("hello world")
    • old : 171.274 us
    • new : 11.920 us
  • 1024 byte + "\r\n"
    • old : 13.999962 ms
    • new : 1.150692 ms
  • 1024 * 10 byte + "\r\n"
    • old : 139.649294 ms
    • new : 11.612682 ms

interrupt.Disable() cannot be built unless interp-rewrite is merged.

Samd2x can be made faster in the same way.
Once we have reviewed this PR, we will create a PR.

@sago35
Copy link
Member Author

sago35 commented Nov 4, 2020

The reason why it is set to draft is that we need to merge the interp-rewrite branches to be able to build.
I would like a review.

I tested it with the following code with WioTerminal.

package main

import (
	"fmt"
	"machine"
	"time"
)

var buf = [1024 * 10]byte{}

var (
	debugPort1 = machine.BCM3
)

func main() {
	debugPort1.Configure(machine.PinConfig{Mode: machine.PinOutput})

	for i := range buf {
		if ((i + 1) & 0x0F) == 0 {
			buf[i] = byte(fmt.Sprintf("%X", byte(i>>4)&0xF)[0])
		} else {
			buf[i] = '.'
		}
	}
	buf[0] = '+'

	for {
		debugPort1.High()

		machine.UART0.Write(buf[:1024*10])
		//machine.UART0.Write(buf[:1024])
		machine.UART0.Write([]byte("\r\n"))
		//fmt.Printf("hello world\r\n")
		//println("hello world")

		debugPort1.Low()
		time.Sleep(time.Second)
	}
}

@sago35 sago35 requested a review from deadprogram November 4, 2020 12:24
@sago35
Copy link
Member Author

sago35 commented Nov 4, 2020

  • Use udd_ep_in_cache_buffer[usb_CDC_ENDPOINT_IN][128] as two 64 byte buffers.
    • The term udd_ep_in_cache_buffer is abbreviated to buffer below
  • WriteByte()
    • Set c byte to buffer[TxIdx].
    • Can write up to 63 bytes.
    • The reason it's 63 bytes instead of 64 bytes is because TxIdx & 0x3F treats it as a size
    • Call Flush() if you've written to the maximum
  • Flush()
    • Flush() sends the data in the buffer
    • Calculate the size to be sent using TxIdx & 0x3F
    • When the size is greater than 0, it is actually sent to USBCDC
    • After sending, set TxIdx to the next buffer
      • For example, when TxIdx is 0 to 63, TxIdx is set to 64. And Set TxIdx to 0 when the value is 64 to 127.
    • Set UART0.sent to true if it is sent once.
      • If not using UART0.sent, USBCDC status is not known at WriteByte() time.
  • handleUSBIRQ()
    • Interrupts every 1ms
    • Calling Flush()

@sago35 sago35 force-pushed the improve-usbcdc-07-samd5x branch from 89002b6 to 15ccc47 Compare February 14, 2021 03:06
@sago35 sago35 marked this pull request as ready for review February 14, 2021 03:10
@sago35
Copy link
Member Author

sago35 commented Feb 14, 2021

It is buildable on the current dev branch so it is ready for review.

I wish I could make the implementation more straightforward, but I can't think of a good way to do it right now.

The code for testing can be found at
#1481 (comment)

I checked it on Windows 10 and found the following

  • 1024 * 10 byte + "\r\n"
    • old : 514.359 ms
    • new : 11.633ms

@deadprogram
Copy link
Member

The code does appear to work. I made a few comments about using const values for clarity.

@aykevl do you have any feedback?

@sago35
Copy link
Member Author

sago35 commented Feb 15, 2021

Updated. @deadprogram

@deadprogram
Copy link
Member

Made a few small comments @sago35 that look like we can shorten by a couple of lines of code.

@sago35
Copy link
Member Author

sago35 commented Feb 15, 2021

Updated. @deadprogram

I thought USBCDC.Flush() should not be changed.
Should I change it?

@deadprogram
Copy link
Member

I think this is now fine how it is. @sago35 can you please squash into a single commit? Thank you!

@sago35 sago35 force-pushed the improve-usbcdc-07-samd5x branch from eefd589 to 1a202f4 Compare February 15, 2021 23:42
@sago35
Copy link
Member Author

sago35 commented Feb 15, 2021

squashed. @deadprogram

@deadprogram
Copy link
Member

This is really great, thank you for working on it @sago35 just waiting for CI to finish to merge.

Now we just need the same thing for SAMD21! 😸

@sago35
Copy link
Member Author

sago35 commented Feb 15, 2021

I will prepare the samd21 version as well.

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