Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix variable naming and extraneous null characters in string#152

Closed
tcwan wants to merge 7 commits intoschodet:masterschodet/nxt-python:masterfrom
tcwan:mastertcwan/nxt-python:masterCopy head branch name to clipboard
Closed

Fix variable naming and extraneous null characters in string#152
tcwan wants to merge 7 commits intoschodet:masterschodet/nxt-python:masterfrom
tcwan:mastertcwan/nxt-python:masterCopy head branch name to clipboard

Conversation

@tcwan
Copy link
Contributor

@tcwan tcwan commented Jun 14, 2018

  1. brick_name_str not referenced correctly in the previous fix.
  2. brick_name_str had extra null characters in the unicode string that needs to be removed, otherwise subsequent strings are not displayed.
  3. Misc source file whites pace cleanups

brick_name_str = brick_name_b.decode('windows-1252')
print("brick_name_b = {0}, brick_name_str = {1}".format(brick_name_b, brick_name_str))

brick_name_str = brick_name_b.decode('windows-1252').strip('\00')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that brick.get_device_info() should return the decoded and stripped value rather than all callers having to do this themselves.

As I recall, the last time I was testing this, it was in fact doing exactly that (tested on Linux and Mac), so I'm not sure why this change would be needed. (I guess I'm going to have to just try it again if I really what to know).

Copy link
Contributor Author

@tcwan tcwan Jun 14, 2018

Choose a reason for hiding this comment

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

Hmm. I tested this using nxt-python 2.2.2 which is not the latest master version.
Maybe that is the problem I'm facing, and we need to have a new release version to test with.

Edit: I'm not sure how to test a WIP python library without installing it to the default python library path.

Copy link
Contributor

Choose a reason for hiding this comment

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

use the PYTHONPATH environment variable

Copy link
Contributor

Choose a reason for hiding this comment

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

we need to have a new release version to test with.

Yes, I made this request recently as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will the new release still be python2.7 compatible?
Python 2 is still the default python interpreter for most OSes.

Copy link
Contributor

Choose a reason for hiding this comment

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

according to the readme of this project, no, it will not be python2 compatible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. So in that case, I'll still need to maintain my own version of nxtfilemgr for 2.2.2.

@tcwan
Copy link
Contributor Author

tcwan commented Jun 15, 2018

I guess I'll close this pull request since it is not really needed due to nxt-python updates for python3 compatibility.
Ongoing work on nxtfilemgr should track nxt_filer enhancement

@tcwan tcwan closed this Jun 15, 2018
@tcwan tcwan mentioned this pull request Jun 27, 2018
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.