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

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

Merged
merged 1 commit into from May 17, 2018

Conversation

Projects
None yet
6 participants
@jamesbeyond
Contributor

jamesbeyond commented May 10, 2018

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

@cmonr cmonr requested review from bridadan and theotherjimmy May 11, 2018

@cmonr cmonr added the needs: review label May 11, 2018

@bridadan

This comment has been minimized.

Contributor

bridadan commented May 11, 2018

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

@mazimkhan

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

@mazimkhan

This comment has been minimized.

Contributor

mazimkhan commented May 14, 2018

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

Sorry for the late reply. The change is good.

@0xc0170

This comment has been minimized.

Member

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)

@0xc0170

LGTM the changes

requesting commit msg update

@0xc0170 0xc0170 added needs: work and removed needs: review labels May 14, 2018

fix a bug in greentea-client
reset "LastChar" after "tok_close" received
fix the bug where greentea-client require a character input between K-V pairs

@jamesbeyond jamesbeyond force-pushed the jamesbeyond:greentea_fix branch from 7966cb3 to d48d3af May 14, 2018

@jamesbeyond

This comment has been minimized.

Contributor

jamesbeyond commented May 14, 2018

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

Changes addressed

@bridadan

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

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 14, 2018

/morph build

@mbed-ci

This comment has been minimized.

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

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr added ready for merge and removed needs: CI labels May 15, 2018

@cmonr cmonr merged commit 54ac02b into ARMmbed:master May 17, 2018

13 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 845 warnings
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 8649 cycles (-368 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 9964B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label May 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment