Skip to content

Conversation

@rosen-vladimirov
Copy link
Contributor

In case the device identifier contains only numeric symbols or in case it looks like a number (16089e09 for example), CLI fails to work with it.
The problem is that CLI thinks an index is passed to it, so it removes "one" from the passed number and tries to find the device on the new index.
The other issue is with the isNumber helper method which works incorrectly when the string looks like number. Improve the logic in the method and rename it. We cannot parse safely values with exponent (1e6 for example), but we do not need such functionality.
The new method name and tests reflects the described behavior.
Before using the identifier as an index, check if we'll find device with the specified identifier.

PR Checklist

What is the current behavior?

When device with numeric identifier is attached, CLI fails to work with it.

What is the new behavior?

CLI can work with devices with numeric identifiers.

Fixes #3832

In case the device identifier contains only numeric symbols or in case it looks like a number (16089e09 for example), CLI fails to work with it.
The problem is that CLI thinks an index is passed to it, so it removes "one" from the passed number and tries to find the device on the new index.
The other issue is with the `isNumber` helper method which works incorrectly when the string looks like number. Improve the logic in the method and rename it. We cannot parse safely values with exponent (`1e6` for example), but we do not need such functionality.
The new method name and tests reflects the described behavior.
Before using the identifier as an index, check if we'll find device with the specified identifier.
@rosen-vladimirov rosen-vladimirov added this to the 4.2.4 milestone Sep 17, 2018
@rosen-vladimirov rosen-vladimirov self-assigned this Sep 17, 2018
@rosen-vladimirov rosen-vladimirov merged commit 0e8aafa into release Sep 17, 2018
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/fix-getting-device-num-identifier branch September 17, 2018 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants