terjeio / grblHAL

This repo has moved to a new home https://github.com/grblHAL
231 stars 90 forks source link

grblHAL/drivers/STM32F1xx/Src/serial.c inverted test #336

Closed JoachimF closed 2 years ago

JoachimF commented 2 years ago

Hello,

Is the test if tx buffer is empty is not inverted ? I'm not an expert but I tested this code outside Grbl and think the test is inverted. USART_SR_TXE is set when buffer is emtpy, so ready to send, we should send c without buffering, let me know. The symptom in my code is that when buffer is empty, some caracters are missing.

// // Attempt to send a character bypassing buffering // inline static bool serialPutCNonBlocking (const char c) { bool ok;

if((ok = !(USART1->CR1 & USART_CR1_TXEIE) && !(USART1->SR & USART_SR_TXE))) should be
if((ok = !(USART1->CR1 & USART_CR1_TXEIE) && (USART1->SR & USART_SR_TXE)))
    USART1->DR = c;

return ok;

}

terjeio commented 2 years ago

I'm not an expert but I tested this code outside Grbl and think the test is inverted.

You are right, but the code is unused and it should be removed as this MCU does not have an on-chip UART FIFO. Adding a call to this function, even when inline, is a waste of cycles. IIRC the idea behind, and possibly initial code, for this function originates from my UART implementation for one of the TI MCUs that do have a FIFO.

JoachimF commented 2 years ago

I use the uart instead of the CDC, I'm trying to make a DRO with 7seg and color LCD and rotary encoder, to jog and probe, I didn't understand how the MPG stream works, so I test with the DRO conected in USB and the grbl connected to the DRO with RS232 -> RS485 -> RS232 to avoid noise.

it's called here bool serialPutC (const char c) { if(txbuf.head != txbuf.tail || !serialPutCNonBlocking(c)) { // Try to send character without buffering...

    uint16_t next_head = BUFNEXT(txbuf.head, txbuf);    // .. if not, get pointer to next free slot in buffer

    while(txbuf.tail == next_head) {                    // While TX buffer full
       // if(!hal.stream_blocking_callback())             // check if blocking for space,
       //     return false;                               // exit if not (leaves TX buffer in an inconsistent state)
    }

    txbuf.data[txbuf.head] = c;                         // 

Add data to buffer, txbuf.head = next_head; // update head pointer and USART1->CR1 |= USART_CR1_TXEIE; // enable TX interrupts }

return true;

}

IMG_20211201_230319