Skip to content
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

Use latest JLink installation DLL, not the oldest #19

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MarekNovakNXP
Copy link
Contributor

In our use case, a problem occurred when using pylink, since it uses the first DLL found in C:\Program Files (x86)\SEGGER. This patch makes it use the newest one.

I think it is legit to suppose, that the newest version of JLink DLL is the less buggy and has the greatest support.

Copy link
Contributor

@hkpeprah hkpeprah left a comment

Choose a reason for hiding this comment

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

I like the idea behind this, but I think it needs a bit more work. I'd want the behaviour to be consistent across Windows/Linux/Mac. There's some tests around this, and comments about how the naming convention has changed from version-to-version and platform-to-platform, so we'd want a nice way of figuring out what is what version, then returning the newest versioned one we found. That way behaviour would be consistent, whereas os.listdir isn't.

lib_path = os.path.join(dir_path, jlink_dir, dll)
if os.path.isfile(lib_path):
yield lib_path
lib_path = os.path.join(dir_path, ds[-1], dll) # use the latest jlink DLL (ds[-1])
Copy link
Contributor

Choose a reason for hiding this comment

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

So this won't necessarily return the newest version. os.listdir returns the directories in the order specified by the underlying filesystem. In some cases, this is just the latest directory created, which is generally the latest installed version, but can be an older version if someone switches between versions often.

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 agree... However I am not sure I have the bandwidth and the environment to test it also on Linux and Mac.
A logic behavior would be to make the user either to pic a specific version and as default, if nothing is specified, use the latest.

Maybe somebody can improve this and provide implementation for Mac and Linux?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yah, I like the idea of that behaviour. The reason why I said that listdir might not be the best approach is that the order isn't guaranteed, so if we say that we always return the latest one, then we have to do some sorting in addition to listdir. To sort properly, we'll need to handle the different file naming conventions that have been had over time.

@sstallion
Copy link
Contributor

sstallion commented Feb 13, 2018 via email

@hkpeprah
Copy link
Contributor

I believe so. Either allowing a specific version or using the latest version will require the same code changes; some way of mapping to versions, given that the naming conventions have changed over time. So one will give us the other.

@sstallion
Copy link
Contributor

sstallion commented Feb 26, 2018 via email

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

4 participants