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

timeout_per_mb does not scale if size < 1e6 (ESF-201) #131

Closed
haydenroche5 opened this issue Jan 16, 2025 · 5 comments
Closed

timeout_per_mb does not scale if size < 1e6 (ESF-201) #131

haydenroche5 opened this issue Jan 16, 2025 · 5 comments
Labels
Type: Bug Bug in esp-serial-flasher

Comments

@haydenroche5
Copy link

Port

ESP

Target chip

ESP32S3

Hardware Configuration

N/A

Log output

N/A

More Information

Context: https://discuss.blues.com/t/outboard-dfu-failure-with-larger-binaries/1947/9?u=hroche

Proposed fix:

static uint32_t timeout_per_mb(uint32_t size_bytes, uint32_t time_per_mb)
{
    uint32_t timeout = ((uint64_t) time_per_mb * size_bytes) / 1000000;
    return MAX(timeout, DEFAULT_FLASH_TIMEOUT);
}
@haydenroche5 haydenroche5 added the Type: Bug Bug in esp-serial-flasher label Jan 16, 2025
@github-actions github-actions bot changed the title timeout_per_mb does not scale if size < 1e6 timeout_per_mb does not scale if size < 1e6 (ESF-201) Jan 16, 2025
@Dzarda7
Copy link
Collaborator

Dzarda7 commented Jan 16, 2025

Hi @haydenroche5, thanks for noticing this. I will fix that. You also need a new release of esp-serial-flasher or you okay with master?

@haydenroche5
Copy link
Author

Master's good. Thank you!

@Dzarda7
Copy link
Collaborator

Dzarda7 commented Jan 16, 2025

I took a closer look now and I cannot agree with you, if I did not miss something. When size_bytes is divided by 1e6 (which is float), it is cast to float, then multiplied by time_per_mb (output is also float) and after that converted to uint32_t. You can try it here, where I prepared the exact example.

@haydenroche5
Copy link
Author

Ah, you are right. I had changed 1e6 to 1000000 and that of course causes the problem I was seeing. Sorry for the false alarm.

@Dzarda7
Copy link
Collaborator

Dzarda7 commented Jan 16, 2025

No problem, if you find the bug somewhere else, feel free to open the issue.

@Dzarda7 Dzarda7 closed this as not planned Won't fix, can't repro, duplicate, stale Jan 16, 2025
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