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

Upstream change in the mac os directory name breaks pico-python #143

Open
ghost opened this issue Oct 22, 2018 · 5 comments
Open

Upstream change in the mac os directory name breaks pico-python #143

ghost opened this issue Oct 22, 2018 · 5 comments

Comments

@ghost
Copy link

ghost commented Oct 22, 2018

looks like in a recent update Pico have, in their wisdom, decided to add a space to the directory name of the mac os version. So now it is /Applications/PicoScope 6.app/Content rather than /Applications/PicoScope6.app/Content. this breaks the darwin_utils

@hmaarrfk
Copy link
Collaborator

Hi @bongotrench,

Did you try to report this change to picoscope itself? This seems kinda annoying, and something they should probably revert in my opinion. If you did, please include a link to their forum.

@hsmistry is there a better way to refer to the location of the DLL in macs?

I don't have a mac, nor do I have a picoscope anymore.
It is also really hard to write unit tests for this kind of code, so I'm hesitant to write something without being able to test it locally.

A PR would be welcome 🙏! Please ensure that it would work under both types of installations (new and old). I understand that it is difficult to test the "old" installation for you, but we will try to "stare and debug" approach.

Please include a comment in the code referring to this issue.

Thanks!

@hmaarrfk
Copy link
Collaborator

Since we wrote this package, picotech seems to have started to support python.

They also picked the ctypes approach.

You can check their code, maybe get inspired
https://github.com/picotech/picosdk-python-wrappers/blob/e8deec9aafec96e102d96e40ee05b562f6fba529/picosdk/library.py#L14

@hsmistry
Copy link

@hmaarrfk, we are looking into how to best provide dylib files for macOS.

Apologies if you were unaware of the change - I will flag this up with a colleague in the Development Team.

You can copy the required .dylib files to another location and add the location to the DYLD_LIBRARY_PATH variable e.g.

export DYLD_LIBRARY_PATH=<path to dylib files>:$DYLD_LIBRARY_PATH

@bongotrench, please do take a look at our new repo

@ghost
Copy link
Author

ghost commented Oct 22, 2018

It can be fixed by just adding that space to the path variable, but that would break it for anyone using the old version of the Picoscope software. Is that OK?

@hmaarrfk
Copy link
Collaborator

@bongotrench sorry, that kind of fix would be unacceptable. I suspect many people might have a "1 version old" SDK where that space doesn't exist.

You can do something like have a loop that goes through the different possible installation paths until something is imported correctly.

Psuedocode

for p in valid_paths:
    try:
        import dyn
    except ImportError: # or the appropriate error
        pass
else:
    raise ImportError('Could not find picoscope library in paths {}.'format(valid_paths))

I can't test this, so I won't write it myself. When you write this, make sure the "old path" gets checked first. This is our "indirect" test of that logic.

@hsmistry Thank you for replying here I know these are not your "official" communication channels. I'm no longer invested in this, but I think this cosmetic change has much more possibility of breaking people's build scripts than it has in improving the readability of the Path. Adding a space in paths is also very annoying to debug as the command line can probably interpret it as two parameters. As an other developer, I would have advised against this kind of change. Now that it has been released, I am not sure what the best approach is.

@hmaarrfk hmaarrfk changed the title mac os directory name Upstream change in the mac os directory name breaks pico-python Oct 22, 2018
jpsecher added a commit to jpsecher/pico-python that referenced this issue Mar 25, 2020
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

No branches or pull requests

2 participants