Skip to content

Conversation

@facchinm
Copy link
Contributor

@facchinm facchinm commented Jun 5, 2025

Tested with Giga (zephyr) and UNO R4

@facchinm facchinm requested a review from eigen-value June 5, 2025 13:12
@facchinm
Copy link
Contributor Author

facchinm commented Jun 5, 2025

@pennam

Copy link
Collaborator

@eigen-value eigen-value left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree on the general PR, but some changes need to be reverted because the library is still under test in a threading scenario

while (!decoder.get_response(msg_id, result, error)){
decoder.process();
delay(1);
if (!decoder.process()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behaviour is still under test in a multithreading context. The delay here is actually a placeholder for a thread yield.
If the client doesn't yield every time its waiting response is not ready, then it can slow down another thread (eg server).
It's better to leave everything as it was and make a decision when we discuss about if/how threading is involved

@per1234 per1234 added type: imperfection Perceived defect in any part of project topic: code Related to content of the project itself labels Jun 8, 2025
@eigen-value eigen-value changed the base branch from main to refactoring_opt June 12, 2025 08:44
@eigen-value
Copy link
Collaborator

I'm merging this as is and do the little revert later

@eigen-value eigen-value merged commit 92c2212 into refactoring_opt Jun 12, 2025
20 checks passed
@eigen-value eigen-value deleted the speedup branch June 27, 2025 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants