Skip to content

Changing the key-value pair regex to include searches for new line #104

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

Merged
merged 1 commit into from
Jun 23, 2016

Conversation

bridadan
Copy link
Contributor

This is in response to the ongoing discussion about this DAPLink issue: ARMmbed/DAPLink#115

The behavior that reveal the issue in DAPLink was the following:

  1. The device sends a key value pair: {{key;value}}\n
  2. The host receives the serial data character-by-character.
  3. KiViBufferWalker matches a key-value pair with the regex \{\{([\w\d_-]+);([^\}]+)\}\}, which does not wait for the new the newline of the key-value pair: {{key;value}} (note no \n).
  4. The host immediately sends a new key-value pair to the device. At this time the device is still sending the \n character.
  5. The device is now receiving a character and attempting to transmit a character at the same time. The particular timing of this case causes a race conditions, which puts DAPLink into a bad state.

This patch forces the host test runner to wait for the \n character before matching any key-value pairs. This doesn't prevent every case where DAPLink might get into a bad state, but it avoids the particular case described above.

@PrzemekWirkus @mazimkhan @adbridge

For more intimate details on the DAPLink issue, please see Russ's commit message on his proposed patch here: ARMmbed/DAPLink@1dcc1b1

@PrzemekWirkus
Copy link
Contributor

@bridadan Brian! Yes, whatever reasons are for DAPLink to fail in this particular case it should be clear how we define K-V protocol. In greetea-client implementation we DO send \n at the end of every key-value message, see here. So it would make perfect sense to align K-V parser on host side with the same newline requirement (vide this PR).
I need to revert ba8c827 and check if we have improvement on K-V protocol !

@PrzemekWirkus PrzemekWirkus merged commit e091413 into ARMmbed:master Jun 23, 2016
PrzemekWirkus added a commit that referenced this pull request Jun 23, 2016
Toghther with #104 should significantly improve e.g, mbed_drivers::echo tests
@PrzemekWirkus
Copy link
Contributor

Add #105 to provide solution for issues Brian mentioned.

@bridadan Please have a look and let me know how it works for you!

PrzemekWirkus referenced this pull request Jun 23, 2016
Simplifies logger (no need to send \n and logger will not log extra line)
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.

2 participants