Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect remaining time calculation (ESF-179) #121

Open
nikitos1550 opened this issue Nov 15, 2024 · 8 comments
Open

incorrect remaining time calculation (ESF-179) #121

nikitos1550 opened this issue Nov 15, 2024 · 8 comments
Labels
Type: Bug Bug in esp-serial-flasher

Comments

@nikitos1550
Copy link

Port

raspberry

Target chip

ESP32S3

Hardware Configuration

ESP32-S3-WROOM-1U based pcba connected via UART to custom mdm9207 (GNU/Linux OS) based pcba.

Log output

Port '/dev/ttyHSL2'
Default baudrate 115200

--- WRITE ---
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 read_char: cannot read byte because of timeout
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 read_char: cannot read byte because of timeout
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 
--- READ ---
c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 
--- WRITE ---
c0 00 14 00 00 00 00 00 00 c0 
--- READ ---
c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 read_char: cannot read byte because of timeout

--- WRITE ---
c0 00 0a 04 00 00 00 00 00 00 10 00 40 c0 
--- READ ---
00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 read_char: cannot read byte because of timeout
Cannot connect to target. Error: 2

--- WRITE ---
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 
--- READ ---
00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 14 18 00 07 07 12 20 00 00 00 00 00 00 00 00 00 00 00 0c 09 00 00 00 00 00 00 00 00 00 00 00 c0 00 00 c0 01 0a 04 00 09 00 00 00 00 00 00 00 c0 00 00 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 read_char: cannot read byte because of timeout

--- WRITE ---
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 
--- READ ---
08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 read_char: cannot read byte because of timeout

--- WRITE ---
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 
--- READ ---
08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 read_char: cannot read byte because of timeout

--- WRITE ---
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 
--- READ ---
08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 read_char: cannot read byte because of timeout

--- WRITE ---
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 
--- READ ---
08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 read_char: cannot read byte because of timeout

--- WRITE ---
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 
--- READ ---
08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 read_char: cannot read byte because of timeout

--- WRITE ---
c0 00 08 24 00 00 00 00 00 07 07 12 20 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 c0 
--- READ ---
08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 08 04 00 07 07 12 20 00 00 00 00 c0 c0 01 read_char: cannot read byte because of timeout

More Information

https://github.com/espressif/esp-serial-flasher/blob/f1183069e23ee89dbab9b411724e88e170dbd874/port/raspberry_port.c#L292C1-L297C1

void loader_port_start_timer(uint32_t ms)
{
    s_time_end = clock() + (ms * (CLOCKS_PER_SEC / 1000));
}

uint32_t loader_port_remaining_time(void)
{
    int64_t remaining = (s_time_end - clock()) / 1000;
    return (remaining > 0) ? (uint32_t)remaining : 0;
}

Here if (s_time_end - clock()) will be less then 1000, then we still have some time to wait, but function will return 0.

I locally patched it

void loader_port_start_timer(uint32_t ms)
{
#if SERIAL_FLASHER_DEBUG_TRACE
    printf("%s: ms = %u\n", __func__, ms);
#endif
    s_time_end = ((clock() / CLOCKS_PER_SEC) /*seconds*/ * 1000)/*ms*/ + ms;
}

uint32_t loader_port_remaining_time(void)
{
        int r;
        int64_t remaining = (s_time_end - ((clock() / CLOCKS_PER_SEC) /* seconds*/ * 1000 /* ms */) );
        r = (remaining > 0) ? (uint32_t) remaining : 0;

//#if SERIAL_FLASHER_DEBUG_TRACE
//      if (r == 0) {
//              printf("%s: r = %u\n", __func__, r);
//      }
//#endif

        return r;
}

And now I can sucessfully burn esp32s3. Seems I faced some hardware compatibility issue, because our previous ESP32-S3-WROOM-1U based pcba did not have such issue, anyway I think my point is reasonable.

@nikitos1550 nikitos1550 added the Type: Bug Bug in esp-serial-flasher label Nov 15, 2024
@github-actions github-actions bot changed the title incorrect remaining time calculation incorrect remaining time calculation (ESF-179) Nov 15, 2024
@Dzarda7
Copy link
Collaborator

Dzarda7 commented Nov 16, 2024

Hi @nikitos1550, thanks for noticing this. You are right, calculation is incorrect. This reminded me the already opened issue here. Your solution fixes the issue you talk about, but another problem is that clock() function measures time consumed by program, real time might be a bit different. Are you willing to open pull request with the solution or you want us to take care of it? If you are not, I will try to fix it soon, just so you know that timeout with clock() function might be a bit longer.

@nikitos1550
Copy link
Author

nikitos1550 commented Nov 17, 2024

Hi @nikitos1550, thanks for noticing this. You are right, calculation is incorrect. This reminded me the already opened issue here. Your solution fixes the issue you talk about, but another problem is that clock() function measures time consumed by program, real time might be a bit different. Are you willing to open pull request with the solution or you want us to take care of it? If you are not, I will try to fix it soon, just so you know that timeout with clock() function might be a bit longer.

Hi! You are right about clock(), so I reworked it following way (added https://github.com/solemnwarning/timespec to submodules )

static struct timespec s_time_end;
// ...
void loader_port_start_timer(uint32_t ms)
{
    struct timespec now, add;

    clock_gettime(CLOCK_MONOTONIC, &now);
    add = timespec_from_ms(ms);
    s_time_end = timespec_add(now, add);
}

uint32_t loader_port_remaining_time(void)
{
    struct timespec now, diff;

    clock_gettime(CLOCK_MONOTONIC, &now);

    if (timespec_ge(s_time_end, now)) {
        diff = timespec_sub(s_time_end, now);
        return timespec_to_ms(diff);
    }

    return 0;
}

If you are ok with additional submodule dep I can make PR.

@nikitos1550
Copy link
Author

anyway here you are

@Dzarda7
Copy link
Collaborator

Dzarda7 commented Nov 17, 2024

I will close the PR and implement it directly without submodule if you are OK with that. Anyway thanks for noticing.

@nikitos1550
Copy link
Author

nikitos1550 commented Nov 17, 2024

I will close the PR and implement it directly without submodule if you are OK with that. Anyway thanks for noticing.

I am ok, sure! I am using respberry based Linux port with custom utility, so it make no sense for me.

@nikitos1550
Copy link
Author

nikitos1550 commented Nov 21, 2024

I will close the PR and implement it directly without submodule if you are OK with that. Anyway thanks for noticing.

Please also pay attention to open tty dev file flags, there is NONBLOCK flag that means VMIN, VTIME options will not work.
Today I also got issue on some device samples, after more detailed debug I found that start timer, remaining timer does not cover all issues (as I thought before), It comes out that during read VTIME is setuped (each time), but as port opened NONBLOCKING, it does not work. I removed flag and it become much better. But I still expirience issue when first esp32-serial connection attempt does not work, need run it twice, something about tty port options I suppose, but I cannot figure out what exactly.

P.S. I will provide log later, maybe you can help me figure out what is going on.

@nikitos1550
Copy link
Author

I cannot reproduce it right now :( Maybe later I will have luck.

@Dzarda7
Copy link
Collaborator

Dzarda7 commented Nov 21, 2024

Thanks for letting me know. It is planned to rewrite the Raspberry pi port completely in the future. Feel free to send the log later, I can take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bug in esp-serial-flasher
Projects
None yet
Development

No branches or pull requests

2 participants