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.

Sign up for GitHub

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

Jump to bottom

Socket HAL bugfix (virtual device) #1134

Merged
merged 2 commits into from
Oct 17, 2016
Merged

Socket HAL bugfix (virtual device) #1134

merged 2 commits into from
Oct 17, 2016

Conversation

sergeuz
Copy link
Member

@sergeuz sergeuz commented Oct 5, 2016 •
edited by technobly
Loading

This PR fixes seemingly random cloud connection errors on virtual device, caused by incorrect error handling in socket_receive(): socket hal uses global ec variable for exceptionless error handling, and previously any error on unrelated socket (such as socket used for multicast announcement) might affect cloud connection.

Doneness:

  • Contributor has signed CLA
  • Problem and Solution clearly stated
  • Code peer reviewed
  • API tests compiled
  • Run unit/integration/application tests on device
  • Add documentation (N/A)
  • Add to CHANGELOG.md after merging (add links to docs and issues)

BUGFIX

  • [GCC Virtual Device] Error in socket_hal's socket_receive() logic caused random cloud connection errors. PR #1134

@sergeuz sergeuz added this to the 0.7.x milestone Oct 5, 2016
@sergeuz sergeuz changed the title Fixed error handling Socket HAL bugfix (virtual device) Oct 5, 2016
@monkbroc monkbroc self-assigned this Oct 11, 2016
monkbroc
monkbroc reviewed Oct 11, 2016
View reviewed changes
Copy link
Member

@monkbroc monkbroc left a comment

Choose a reason for hiding this comment

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

I see the { added to the if statement, but I'm not sure how that relates to the ec global variable. Is it because without the { the previous value of ec is checked even when the if condition is false, leading to false errors?

@sergeuz
Copy link
Member Author

sergeuz commented Oct 11, 2016

Yes, exactly

@monkbroc
Copy link
Member

Must have { on all ifs!

@monkbroc monkbroc removed their assignment Oct 14, 2016
@technobly technobly merged commit 801772e into develop Oct 17, 2016
@technobly technobly deleted the fix/gcc_socket_hal branch October 17, 2016 01:45
@technobly technobly modified the milestones: 0.7.x, 0.6.1 Nov 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@monkbroc monkbroc monkbroc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.6.1
Development

Successfully merging this pull request may close these issues.
3 participants
@sergeuz @monkbroc @technobly