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

How to call mavgen? #813

Open
julianoes opened this issue May 8, 2023 · 11 comments · May be fixed by #838
Open

How to call mavgen? #813

julianoes opened this issue May 8, 2023 · 11 comments · May be fixed by #838

Comments

@julianoes
Copy link
Contributor

julianoes commented May 8, 2023

I'm confused what the right way should be to call mavgen.

According to various docs, I would assume the canonical way should be to install pymavlink via pip:

python -m pip install pymavlink  
Defaulting to user installation because normal site-packages is not writeable
Collecting pymavlink
  Using cached pymavlink-2.4.38-py3-none-any.whl (11.3 MB)
Requirement already satisfied: lxml in /usr/lib/python3/dist-packages (from pymavlink) (4.8.0)
Requirement already satisfied: future in ./.local/lib/python3.10/site-packages (from pymavlink) (0.18.3)
Installing collected packages: pymavlink
Successfully installed pymavlink-2.4.38

And then run it like this:

python -m pymavlink.tools.mavgen
/usr/bin/python: Error while finding module specification for 'pymavlink.tools.mavgen' (ModuleNotFoundError: No module named 'pymavlink.tools')

However, that doesn't seem to work. It doesn work when pymavlink is in tree, like when in the mavlink repo where pymavlink is a submodule, however, I would challenge whether pymavlink really needs to be a submodule there or whether that could be simplified.

@peterbarker any hints? Am I using it wrong?

@JonasVautherin
Copy link
Contributor

It doesn work when pymavlink is in tree

You mean it does work in that case, right?

@julianoes
Copy link
Contributor Author

@JonasVautherin yes thanks. Fixed it.

@julianoes
Copy link
Contributor Author

Or what about adding tools as a package in setup.py, would that work?

diff --git a/setup.py b/setup.py
index 6ccec166..86c97638 100644
--- a/setup.py
+++ b/setup.py
@@ -128,6 +128,7 @@ setup (name = 'pymavlink',
                         'pymavlink'              : ['message_definitions/v*/*.xml']
                         },
        packages = ['pymavlink',
+                   'pymavlink.tools',
                    'pymavlink.generator',
                    'pymavlink.dialects',
                    'pymavlink.dialects.v10',

@julianoes
Copy link
Contributor Author

@peterbarker any opinions on that?

FYI @hamishwillee and @auturgy.

@peterbarker
Copy link
Contributor

mavgen.py in in my path after installing pymavlink.

Pretty sure it's there because it's in the scripts section of setup.py.

It's a wrapper around the generator/mavgen.py library - so if you're looking to call mavgen.py as a library I'd be looking inside tools/mavgen.py.

@julianoes
Copy link
Contributor Author

Yes, it is installed as a script, and if you have e.g. ~/.local/bin/mavgen.py in your path, then that works, but that's generally not something that's easy to explain to users in documentation.

And that's where I believe python -m pymavlink.tools.mavgen would be quite handy. Plus that way is already all over the MAVLink docs but it only actually works using the pymavlink submodule. However, that's where I'm thinking we could simplify it and not have to use the simplify and instead just use it via pip. That's where this questions is coming from.

@hamishwillee
Copy link
Contributor

I though we might be able to do something like adding the following to setup.py, but it still doesn't find mavgen (though it builds cleanly).

entry_points={
           "console_scripts": ["mavgen=pymavlink.tools.mavgen:main",]
       },

@peterbarker It really would be better if we could just do python -m pymavlink.tools.mavgen from anywhere, or simply mavgen Xxxxx. The docs seem to indicate that is possible.

@rotu
Copy link
Contributor

rotu commented Jul 5, 2023

afaict, there was never a deliberate decision to break python -m pymavlink.tools.mavgen and it still will work (without #835) if you install the package in editable mode.

@rotu
Copy link
Contributor

rotu commented Jul 5, 2023

Closing my PR. This package needs much cleanup from someone who knows it better than me. I think pymavlink has too much technical debt right now to comfortably contribute.

Something I noticed is that we're using old-style setup scripts. Switching over to console scripts might fix this (and obviate the nasty hack of adding the parent of the repo root to sys.path).

@hamishwillee hamishwillee linked a pull request Jul 6, 2023 that will close this issue
@hamishwillee
Copy link
Contributor

Thanks @rotu - frustrating.

Still, encouraged me to try the console scripts again: #838 - it "works" but might not be optimal.

@julianoes
Copy link
Contributor Author

I'd suggest to merge #838.

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 a pull request may close this issue.

5 participants