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

Modifications to support Python 3 and wxPython Phoenix (4.0) - WIP #420

Closed
wants to merge 24 commits into from

Conversation

SamuelDudley
Copy link
Contributor

@SamuelDudley SamuelDudley commented Jun 13, 2017

I have opened this WIP to get an idea for what would be required to support wxPython Phoenix and python 3.x. This WIP is a quick hack and contains breaking changes for 2.x Python versions. Tested with Python 3.5 and 4.0.0a3 release of wxPython Phoenix.

  • Windows has pickling errors with the map module (unsure how badly it breaks the map functionality), no issues under linux
  • Currently mission editor does not run due to annoying pickling errors (windows and linux)
  • PyMAVLink is OK under Python3.x, but some of the tools (like DF log reader) do not work
  • Pyserial, Matplotlib, Numpy and OpenCV 3.2 (built from source but a pip package exists) are all OK under Python3.x
  • Not sure how these changes will impact OSX, I need a willing tester.
  • A few of the changes will be compatible with Python2.7. It might be worth isolating them and applying those changes first

@SamuelDudley SamuelDudley changed the title wx widgets - modifications to support wxPython Phoenix (4.0) wx widgets - modifications to support wxPython Phoenix (4.0) - WIP Jun 13, 2017
@stephendade
Copy link
Contributor

Would this be the last thing stopping MAVProxy supporting Python 3.x?

You'd want to double-check that the pymavlink port to Python 3.x has been completed. I'm fairly certain it has though.

All standard libraries other than that should be ok - we'd just have to check they all have Python 3.x packages.

Finally, I suspect they'll be some syntax changes required in MAVProxy itself.

But, WxPython is by far the largest item stopping us from Python 3 compatibility.

Great work :)

@stephendade
Copy link
Contributor

You'd also want to ensure that nothing's broken in the Windows build too

@SamuelDudley
Copy link
Contributor Author

SamuelDudley commented Jun 13, 2017

Unfortunately I don't have access to a windows PC, so I'll have to outsource that particular item :) . However, I will have a go at installing the python 3 versions of everything and see where the remaining issues are. I'll report back here what I find...

@stephendade
Copy link
Contributor

Ahh, OK. Outsourced to me in that case :)

@SamuelDudley
Copy link
Contributor Author

SamuelDudley commented Jun 13, 2017

Alright! :) Machete coding aside, that last commit is everything needed to get all the default modules +map +wxconsole running in python 3.5.
screenshot from 2017-06-14 01-02-56

@SamuelDudley SamuelDudley changed the title wx widgets - modifications to support wxPython Phoenix (4.0) - WIP Modifications to support Python 3 and wxPython Phoenix (4.0) - WIP Jun 13, 2017
@stephendade
Copy link
Contributor

Just quickly tested console and map - they appear to work in Windows. Will do more comprehensive testing later.

@SamuelDudley
Copy link
Contributor Author

Good to hear and thanks for testing :) let me know if you hit any issues. What other GUI elements are currently in the main build / supported?

  • Mission editor
  • Mag cal GUI
  • All the cuav modules
  • Mavexplorer
    Am I missing anything else?

@stephendade
Copy link
Contributor

There's some common stuff like File Open/Save dialogs and menus in mp_menu.py and some of the other files in there.

Also the graph UI, Horizon module, Checklist module.

I have a sneaking suspicion that there'll be some Python2->3 syntax issues in some areas, so you'll be wanting to test all the modules for Python3 compatibility.

We really need a unit test suite :(

@SamuelDudley
Copy link
Contributor Author

So heres an interesting problem I have found. Multiprocessing is not starting child processes during runtime. I can use a launch command like this --master tcp:127.0.0.1:5760 --sitl 127.0.0.1:5501 --out 127.0.0.1:14551 --out 127.0.0.1:14550 --console --map --cmd="module load checklist; module load horizon; module load graph; graph SIMSTATE.roll" and everything is fine, but attempting to start a module which uses multiprocessing (e.g. map, graph, checklist, horizon) with module load XXX will fail silently...
@stephendade try running MAVProxy without the --map argument and then type module load map. On my system the map child process never starts correctly. Really odd...

screenshot from 2017-06-15 12-50-15

if len(cmds) == 1 and cmds[0] == "":
mpstate.empty_input_count += 1
for c in cmds:
process_stdin(c)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

calling module load map here works, but the same command here https://github.com/SamuelDudley/MAVProxy/blob/357239cb97fd6b50d1fc8087b24502bfc75b67fa/MAVProxy/mavproxy.py#L771 fails silently.
It only seems to affect multiprocessing, child threads can run fine.
The only difference between the two locations of code is that this is in the main thread and the other location is in a thread derived from the main thread https://github.com/SamuelDudley/MAVProxy/blob/357239cb97fd6b50d1fc8087b24502bfc75b67fa/MAVProxy/mavproxy.py#L1138 .

@stephendade
Copy link
Contributor

The changes you made since last I tested have broken the Windows build:
capture

When I try running without --console:
capture2

@SamuelDudley
Copy link
Contributor Author

SamuelDudley commented Jun 15, 2017

@stephendade Thanks. Can you try the latest commit when you have a chance please. Windows does not support forkserver, so I have changed the multiprocessing type to spawn.

I have MAVExplorer working but I have had to hack pymavlink DFReader.py as it is not compatible with python3. Modified DFReader.py is here https://gist.github.com/SamuelDudley/a016f6796f2fb448f183301e530ee4b9 (only tested binary logs at this stage)
screenshot from 2017-06-15 21-58-51

@stephendade
Copy link
Contributor

You're quick!

I'm getting errors about urllib2 not being found in the help module (same as before).

Console and map work quite nicely now. MAVProxy completely freezes when I go to the settings dialog and click "cancel".

Get the following error when I click on the map:
capture

Also, nothing happens when a enter "param show RC1_MIN" ... I think the param module is not completely working. "param show all" works though.

@SamuelDudley
Copy link
Contributor Author

Ah thanks. I can fix the params and help. The settings screen is annoying but I can try a few things there. Not sure where to start with the map issue, but I'll have a poke around.

@SamuelDudley
Copy link
Contributor Author

Alright!

  • Param and help modules should be good now. note that pymavlink needs some changes to correctly detect a param has been updated.
  • I have attempted to fix the settings widget. Let me know if that is still broken for you.
  • Lastly I have added some prints to where I think the ImagePanel object is trying to be pickled. Can you please dump the text into a gist or a reply here for me.
    Thanks!

@stephendade
Copy link
Contributor

More results:
-param module now works
-help module has the following error:
capture2

-Settings widget works
-Imagepanel output here:
capture

@@ -22,31 +23,34 @@ def __init__(self, mpstate):
try:
import pkg_resources
self.version = pkg_resources.require("mavproxy")[0].version
except:
# what error occurs here in windows? ImportError?
except ImportError: # we need to catch the explicit error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What error will windows raise here? I'm guessing at ImportError...

@SamuelDudley
Copy link
Contributor Author

SamuelDudley commented Jun 21, 2017

Sorry about the help module... I fixed the param help because I thought thats what you were referring to! I have made some changes to the actual help module but I'm unsure what error windows will raise here. I need to catch the more specific error as the except statement was causing problems for me. I'm running from source and the two existing ways of determining the version of MAVProxy crashed the help module.

Re the ImagePanel pickle... I'm stumped at this point... Any thoughts would be appreciated! :)

@stephendade
Copy link
Contributor

Here's the help module output:
capture

Param help works :)

For the imagepanel pickle - I remember similar errors happening when I was doing the initial Windows conversion for MAVProxy. It happended when multiprocessing attempted to send a variable from one thread to another. In Linux it would just use shared memory, whereas in Windows it would have to package up the variable (via pickle) and send it across (via a queue I think). So most likely cross-threading shenangians.

@SamuelDudley
Copy link
Contributor Author

SamuelDudley commented Jun 21, 2017

That help module error should be fixed for you with the last set of commits. I think your on the right track with the pickling issue... I have spent a few hours attempting to get mission editor to run without success. At the moment I'm facing a range of cryptic pickling errors too :( Unfortunately the stack trace is completely useless...

@stephendade
Copy link
Contributor

I now get a different error when loading up the help module:
capture

@SamuelDudley
Copy link
Contributor Author

SamuelDudley commented Jun 29, 2017

Thanks for testing... I'll have a look at it :)

@SamuelDudley
Copy link
Contributor Author

@stephendade Should be good now.

@stephendade
Copy link
Contributor

Yep, help module works nicely now.

I have discovered another issue. Tab completion appears to be broken:
capture

(I tried tab completion for a variety of modules and settings, same error)

@SamuelDudley
Copy link
Contributor Author

@stephendade Great! Thanks again for testing... I'll have a look at that issue

@njoubert
Copy link
Member

njoubert commented Jul 16, 2017

I am hugely in favor of this PR for one main reason. Mac GUI support is possible with Python 3. I have a demo running on my machine.

@SamuelDudley
Copy link
Contributor Author

SamuelDudley commented Jul 16, 2017

@stephendade tab completion is working on my linux machine... I'll have to chase down the issue based on your stack trace. Please bear with me...
@njoubert when you say that you have a demo running on your machine, are you running the code from this PR or something else?

@SamuelDudley
Copy link
Contributor Author

Im rebasing here https://github.com/SamuelDudley/MAVProxy/tree/wx_phoenix_wip_rebase
I need to test all of this again because there were a few merge conflicts. Once I'm happy I'll update this branch

@SamuelDudley
Copy link
Contributor Author

@amilcarlucas rebased

import traceback
import select
import shlex
import platform
import importlib
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will work for python2.7+ and python3.x

@francisrod01
Copy link

I remember that I try to use with Python 3.6 version but without success.
#468

@tribbloid
Copy link

python2 support should be discarded, it almost reaches end-of-life.

@SamuelDudley
Copy link
Contributor Author

In reality python2 will not go away overnight. I wish I had more time to take some of my changes in this branch and make usable patches to help the maintainers...
If anyone feels up to it it would help out the project quite a bit re python3 support!

@peterbarker
Copy link
Contributor

peterbarker commented Feb 25, 2018 via email

@stephendade
Copy link
Contributor

As an aside, wxPython 4 now appears to support Python 2.7 (https://pypi.python.org/pypi/wxPython/4.0.1), so no issue with GUI toolkits.

I'd really like to resurrect this PR and get it merged in - thoughts?

@SamuelDudley
Copy link
Contributor Author

@stephendade I would love to see python 3.x support merged in but I personally don't have a much time to commit to this PR at the moment, sorry.
The biggest issue the 3.x support work will face IMO are the pickling errors. I have burned quite a few hours chasing them down without any real progress.

@SamuelDudley
Copy link
Contributor Author

If you look at https://github.com/ArduPilot/MAVProxy/pull/420/files you can see that the number of changes is not that huge but it needs to be broken down into a set of PRs rather than this mess

@stephendade
Copy link
Contributor

That's fine. Once I've finished my work with the cuav software, I'll see if I can split this up into manageable PR's and add some unit tests/CI to make the review easier.

@tridge
Copy link
Contributor

tridge commented Oct 15, 2018

i've got a new py3 PR here #560

@SamuelDudley
Copy link
Contributor Author

@tridge are you happy for me to close this PR?

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.

None yet

8 participants