martin2250 commented 6 years ago

Hi,

with my setup (STM32 Blue Pill) some chars sent over USB get corrupted in transmission. eg. I get the occasional [BRB:] instead of PRB, sometimes status messages have some wrong characters (both in text and numbers). I have yet to find an error in the transmission from the PC to grbl.

I'm using a single USB cable which is relatively short, though this shouldn't affect the data anyways as USB normally uses CRC. Is CRC disabled for incoming messages like in vUSB? This would explain why (so far) only messages from the PC have been corrupted.

Martin

robomechs commented 6 years ago

I have the same problem (from grbl -> to PC), but it happens very rarely, and I can't catch it. Most of all I'm worried that a mistake (error) can occur in meaningful numbers, but so far this has not happened.

Sombat4t commented 6 years ago

I've used to have this when I use Universal GCode Sender, but after I've wrote my own Gcode sender this problem less and rarely occured. For me, I think the main problem will be the way to interaction of "send and response protocol" not appropriate. Furthermore; this issue will always exist by its nature.

martin2250 commented 6 years ago

Did you publish this custom streamer somewhere? I'm also using my own software (OpenCNCPilot).

For me, I think the main problem will be the way to interaction of "send and response protocol" not appropriate

sorry, what exactly do you mean by that?

Furthermore; this issue will always exist by its nature.

No, this is absolutely not ok. Error checking and handling should be taken care of by the USB protocol.

Sombat4t commented 6 years ago

No, just starting for test and It is not in practical. But by the way, I make changes some at "usb_endpoint.c" as below. I get in- completed message some for real time, but does not be corrupted. I have no any ideas, but I think the result is better.

volatile uint8_t bTxPackage = 1; void EP1_IN_Callback (void) { bTxPackage = 1; }

void EP1_IN_Callback2(void) { if ((servoid EP1_IN_Callback2(void) { ... ... bTxPackage = 0; SetEPTxCount(ENDP1, USB_Tx_length); SetEPTxValid(ENDP1); ... } }

void SOF_Callback(void) { if(bDeviceState == CONFIGURED) { / Check the data to be sent through IN pipe / if(bTxPackage == 1) { EP1_IN_Callback2(); } } }

2018-06-25 22:34 GMT+07:00 Martin Pittermann notifications@github.com:

Did you publish this custom streamer somewhere? I'm also using my own software (OpenCNCPilot).

For me, I think the main problem will be the way to interaction of "send and response protocol" not appropriate

sorry, what exactly do you mean by that?

Furthermore; this issue will always exist by its nature.

No, this is absolutely not ok. Error checking and handling should be taken care of by the USB protocol.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/usbcnc/grbl/issues/46#issuecomment-399994916, or mute the thread https://github.com/notifications/unsubscribe-auth/AlZzSiTteP-rIOsMwDBvVse6_xOo8R5Hks5uAQLtgaJpZM4UoYcI .

Sombat4t commented 6 years ago

Hi, I think I find the solution for this issue, but no prove. perhaps you want to test on yours. I test on mine. there are no any corrupts or incomplete for real time status message. To do is just lock the "serial_tx_buffer_head" variable in ring buffer as below. Three files added: usb_endp.c, serial.h and serial.c.

1) File usb_endp.c

volatile uint8_t txUsbLock = 0;

void EP1_IN_Callback (void) {}

void EP1_IN_Callback2(void) {
uint16_t USB_Tx_length; uint8_t head;

txUsbLock = 1;
head = serial_tx_buffer_head;
txUsbLock = 0;

if ((head != serial_tx_buffer_tail))
{
    if (head > serial_tx_buffer_tail)
        USB_Tx_length = head - serial_tx_buffer_tail;
    else
        USB_Tx_length = TX_BUFFER_SIZE - serial_tx_buffer_tail + head;

} void SOF_Callback(void) { if(bDeviceState == CONFIGURED) { / Check the data to be sent through IN pipe /
if(_GetEPTxStatus(ENDP1) == EP_TX_NAK) {
EP1_IN_Callback2(); } } }

2) File serial.h extern volatile uint8_t txUsbLock;

3) File serial.c void serial_write(uint8_t data) { ... // Store data and advance head serial_tx_buffer[serial_tx_buffer_head] = data; while(txUsbLock) { if (sys_rt_exec_state & EXEC_RESET) { return; } // Only check for abort to avoid an endless loop. } serial_tx_buffer_head = next_head;

martin2250 commented 6 years ago

@Sombat4t I'll test that as soon as I can. Thanks!

Sombat4t commented 6 years ago

Hi, I test it on Universal Gcode sender. It is my first time to get running 44044 rows( 3:44:23 ) completed and no any errors. By the way, I visit your project. I'd like it. perhaps If possible and don't mind, I'd like to have help in some other time.

2018-06-29 17:00 GMT+07:00 Martin Pittermann notifications@github.com:

@Sombat4t https://github.com/Sombat4t I'll test that as soon as I can. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/usbcnc/grbl/issues/46#issuecomment-401309702, or mute the thread https://github.com/notifications/unsubscribe-auth/AlZzSuL10VBLzajj-vVyVAzHXQRLrQhNks5uBfrFgaJpZM4UoYcI .

martin2250 commented 6 years ago

can you please create a pull request or fork this repo and make your changes to your fork? The code from your comment is incomplete and I can't get it to work.

Sombat4t commented 6 years ago

I confirm it work without any errors. I test on my soft and USG. I think I cannot do pull request my sources changed too many because I re-arrange my sources for to read easily. As my study and my thinking the EP1_IN_Callback is call two times in USB interrupt to gain output so I change it back but modify some and try test it works OK and faster. I will copy you complete sources. If it doesn't work. The problem may be on your side. As I visit yours, I observed. I think it nice and OK. Actually, I'd like to test it but my environment is Linux so not possible.

1) usb/usb_endp.c

volatile uint8_t txUsbLock = 0;

void EP1_IN_Callback(void) { uint16_t USB_Tx_length; uint8_t head;

if(_GetEPTxStatus(ENDP1) != EP_TX_NAK) return;

txUsbLock = 1; // this may be removed, just make sure the local head

variable save the serial_tx_buffer_head before doing condition check head = serial_tx_buffer_head; txUsbLock = 0; // this may be removed

if ((head != serial_tx_buffer_tail))
{
    if (head > serial_tx_buffer_tail)
        USB_Tx_length = head - serial_tx_buffer_tail;
    else
        USB_Tx_length = TX_BUFFER_SIZE - serial_tx_buffer_tail + head;

    if (USB_Tx_length != 0)
    {
        if (USB_Tx_length > 64)
            USB_Tx_length = 64;

//        UserToPMABufferCopy(&serial_tx_buffer[serial_tx_buffer_tail],

ENDP1_TXADDR, USB_Tx_length); { uint8_t pbUsrBuf = serial_tx_buffer + serial_tx_buffer_tail; uint32_t n = (USB_Tx_length + 1) >> 1; / n = (wNBytes + 1) / 2 / uint32_t i; uint16_t temp1; uint16_t pdwVal= (uint16_t )(ENDP1_TXADDR 2 + PMAAddr); for (i = 0; i<n; i++) { temp1 = (uint16_t) pbUsrBuf; pbUsrBuf++; if (pbUsrBuf - serial_tx_buffer == TX_BUFFER_SIZE) pbUsrBuf = serial_tx_buffer; pdwVal++ = temp1 | (uint16_t) * pbUsrBuf << 8; pdwVal++; pbUsrBuf++; if (pbUsrBuf - serial_tx_buffer == TX_BUFFER_SIZE) pbUsrBuf = serial_tx_buffer; } }

        serial_tx_buffer_tail += USB_Tx_length;
        if (serial_tx_buffer_tail >= TX_BUFFER_SIZE)
            serial_tx_buffer_tail -= TX_BUFFER_SIZE;

        SetEPTxCount(ENDP1, USB_Tx_length);
        SetEPTxValid(ENDP1);

    }
}

} / \brief Start Of Frame (SOF) callback / void SOF_Callback(void) { if(bDeviceState == CONFIGURED) { / Check the data to be sent through IN pipe / EP1_IN_Callback(); } } 2) grbl/serial..h extern volatile uint8_t txUsbLock;

3) grbl/serial.c // Writes one byte to the TX serial buffer. Called by main program. void serial_write(uint8_t data) { // Calculate next head uint8_t next_head = serial_tx_buffer_head + 1;

ifdef STM32F103C8

ifndef USEUSB

USART_SendData(USART1, data);
while (!(USART1->SR & USART_FLAG_TXE));         //µÈŽý·¢ËÍÍê³É
return;

endif

if (next_head == TX_RING_BUFFER) { next_head = 0; }
// Wait until there is space in the buffer
while (next_head == serial_tx_buffer_tail)
{
    // TODO: Restructure st_prep_buffer() calls to be executed here

during a long print. if (sys_rt_exec_state & EXEC_RESET) { return; } // Only check for abort to avoid an endless loop.

ifdef WIN32

     Sleep(1);

endif

}

// Store data and advance head
serial_tx_buffer[serial_tx_buffer_head] = data;
while(txUsbLock) { // lock until txUsbLock to 0 on EP1_callback and

change serial_tx_buffer_head variable if (sys_rt_exec_state & EXEC_RESET) { return; } // Only check for abort to avoid an endless loop. } serial_tx_buffer_head = next_head;

ifdef AVRTARGET

// Enable Data Register Empty Interrupt to make sure tx-streaming is

running UCSR0B |= (1 << UDRIE0);

endif

}

2018-07-07 13:56 GMT+07:00 Martin Pittermann notifications@github.com:

can you please create a pull request or fork this repo and make your changes to your fork? The code from your comment is incomplete and I can't get it to work.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/usbcnc/grbl/issues/46#issuecomment-403194107, or mute the thread https://github.com/notifications/unsubscribe-auth/AlZzSitm8adw_T8p9UZIBuFhMoCvUfkSks5uEFu2gaJpZM4UoYcI .

robomechs commented 6 years ago

With this changes it work with OpenCNCPilot. But it works without changes too. Apparently I need more tests))) OpenCNCPilot is an interesting program.

robomechs commented 6 years ago

Error while processing response <<Idle|MPos:o.000,20.000,0.000,0.000|FS:0,0|Pn:XYZA|Ov:100,100,100|A:S>>

Error while processing response <<Idle|MPos:0.P00,20.000,0.000,0.000|FS:0,0|Pn:XYZA>>: For input string: "0.P00"

Error while processing response <<Run|MPos:0.s04,0.004,0.004,0.000|FS:30,0|Pn:XYZA>>: For input string: "0.s04"

Error while processing response <<Run|MPo|:0.005,0.005,0.005,0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPos:0.004,0.004,0,004,0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPo.:0.001,0.001,0.001,0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPos:0.011,0.011,0.011,0.000|FS:30.0|Pn:XYZA|Ov:100,100,100|A:S>>: 1

Error while processing response <<Run|MPos:0.005,00005,0.005,0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPos:0.004,0.004,0.004,0.000|FS:30,0.Pn:XYZA>>: For input string: "0.Pn:XYZA"

Error while processing response <<Run|MPos:0.017,0.017,0.017,0.000|FS:30,:|Pn:XYZA>>: For input string: ":"

Error while processing response <<Run|MPos:0.017,0.017,01017,0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPos:0.012,0.012,0.012,0.000|FS:30P0|Pn:XYZA>>: For input string: "30P0"

Error while processing response <<Run|MPos:81.821,77.277,77.277,0.000|FS:500,07Pn:XYZA>>: For input string: "07Pn:XYZA"

with changes (with UGS) there are no corrupted lines!

Sombat4t commented 6 years ago

On my own soft, I catch all response message, there is also no any errors too. if you have a free time, please do try to remove txUsbLock variable, I think it may be no need. I now don't have spare time to test myself; very busy with my NV40 vert/frag shader assembly to binary code on another subject. I'll be looking forward to it. Thanks in advance.

2018-07-31 15:55 GMT+07:00 yaroslav notifications@github.com:

without changes (with UGS) 2-3 corrupted lines each 1000 "?": Error while processing response <<Idle|MPos:0.000,20.000,0. 000,0.000|FS:0,0|Pn:XYZA|Ov:P00,100,100|A:S>>: For input string: "P00"

Error while processing response <<Idle|MPos:o.000,20.000,0. 000,0.000|FS:0,0|Pn:XYZA|Ov:100,100,100|A:S>>

Error while processing response <<Idle|MPos:0.P00,20.000,0.000,0.000|FS:0,0|Pn:XYZA>>: For input string: "0.P00"

Error while processing response <<Run|MPos:0.s04,0.004,0.004,0.000|FS:30,0|Pn:XYZA>>: For input string: "0.s04"

Error while processing response <<Run|MPo|:0.005,0.005,0.005, 0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPos:0.004,0.004,0,004, 0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPo.:0.001,0.001,0.001, 0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPos:0.011,0.011,0.011, 0.000|FS:30.0|Pn:XYZA|Ov:100,100,100|A:S>>: 1

Error while processing response <<Run|MPos:0.005,00005,0.005, 0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPos:0.004,0.004,0.004,0.000|FS:30,0.Pn:XYZA>>: For input string: "0.Pn:XYZA"

Error while processing response <<Run|MPos:0.017,0.017,0.017,0.000|FS:30,:|Pn:XYZA>>: For input string: ":"

Error while processing response <<Run|MPos:0.017,0.017,01017, 0.000|FS:30,0|Pn:XYZA>>

Error while processing response <<Run|MPos:0.012,0.012,0.012,0.000|FS:30P0|Pn:XYZA>>: For input string: "30P0"

Error while processing response <<Run|MPos:81.821,77.277,77.277,0.000|FS:500,07Pn:XYZA>>: For input string: "07Pn:XYZA"

with changes (with UGS) there are no corrupted lines!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/usbcnc/grbl/issues/46#issuecomment-409147292, or mute the thread https://github.com/notifications/unsubscribe-auth/AlZzSsd01H_UU012cAJcnaoHxQFbbANzks5uMBt9gaJpZM4UoYcI .

usbcnc / grbl

Serial Transmission corrupts some characters #46

ifdef STM32F103C8

ifndef USEUSB

endif

endif

ifdef WIN32

endif

ifdef AVRTARGET

endif