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

Update SFTP status callback to output once per second #779

Merged
merged 8 commits into from
Feb 26, 2025

Conversation

devin-ai-integration[bot]
Copy link
Contributor

Update SFTP status callback to output once per second

Modified the myStatusCb function in sftpclient.c to only output status updates once per second by tracking the last output time and comparing it with the current time. This reduces the frequency of status updates while maintaining all existing functionality.

Testing

The changes have been verified using cppcheck static analysis tool. The modifications maintain the existing functionality while adding rate limiting to the status output.

Requested by

[email protected]

Link to Devin run: https://app.devin.ai/sessions/23b1fd68009a48c8bb03b5309830f193

Modified the myStatusCb function in sftpclient.c to only output status
updates once per second by tracking the last output time and comparing
it with the current time. This reduces the frequency of status updates
while maintaining all existing functionality.

Co-Authored-By: [email protected] <[email protected]>
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add "(aside)" to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@wolfSSL-Bot
Copy link

Can one of the admins verify this patch?

When starting a new file transfer, reset the lastOutputTime to ensure
the first status update for the new file is shown immediately.

Co-Authored-By: [email protected] <[email protected]>
@LinuxJedi
Copy link
Member

(aside) In Linux, on a high-bandwidth connection, this was causing large performance bottleneck.

The lastOutputTime variable is only used when timestamps are enabled,
so it should be guarded by the same macro to avoid unused variable
warnings in builds where timestamps are disabled.

Co-Authored-By: [email protected] <[email protected]>
@LinuxJedi
Copy link
Member

(aside) After explaining to Devin why Zephyr was failing, it fixed it. I think is is ready to review again. Merge will likely need to be a squash merge.

@JacobBarthelmeh
Copy link
Contributor

Ok to test

@JacobBarthelmeh JacobBarthelmeh merged commit 76e8b9f into master Feb 26, 2025
75 checks passed
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