Skip to content

fix greentea-client, require a character input between K-V pairs #6865

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
May 17, 2018

Conversation

jamesbeyond
Copy link
Contributor

Description

Currently, greentea-client require a character input in between 2 K-V pair sync commands.
e.g. between {{__sync;1234}} and {{base_time;0}} in a time drifting test.

This is becasue greentea-client didn't reset the LastChar after it receives "tok_close".
The problem didn't got expose before, becasue in mbedhtrun side, it always insert a line feed "\n" in between any 2 K-V pairs.

in the serial connection, that works fine, cancel out the issue.
However, working with Fast Models, specially in windows enviroment, which use telnet as UART. It which use "\r\n" to flush the buffer, this is causing a problem.

So the fix is to reset "LastChar" after received "tok_close"

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@bridadan
Copy link
Contributor

@mazimkhan any chance you're more familiar with this part of the greentea-client?

Copy link

@mazimkhan mazimkhan left a comment

Choose a reason for hiding this comment

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

Looks good. While we are here can you please fix the indentation of this if block.

@mazimkhan
Copy link

@mazimkhan any chance you're more familiar with this part of the greentea-client?

Sorry for the late reply. The change is good.

@0xc0170
Copy link
Contributor

0xc0170 commented May 14, 2018

Can you update the commit message (contains info from the description - useful info). Plus the first line should be within 50 characters as stated in our guidelines (avoid wrapping, can be seen here on github as well)

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

LGTM the changes

requesting commit msg update

reset "LastChar" after "tok_close" received
fix the bug where greentea-client require a character input between K-V pairs
@jamesbeyond
Copy link
Contributor Author

@0xc0170, Hi Martin, the commit message had been changed

@cmonr cmonr dismissed 0xc0170’s stale review May 14, 2018 16:09

Changes addressed

Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

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

I think it'll be ok, running it through the CI tests should catch any issues.

@cmonr
Copy link
Contributor

cmonr commented May 14, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented May 14, 2018

Build : SUCCESS

Build number : 2010
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6865/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented May 15, 2018

@mbed-ci
Copy link

mbed-ci commented May 15, 2018

@cmonr cmonr merged commit 54ac02b into ARMmbed:master May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants