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

Joy2key improvements #3196

Closed
wants to merge 4 commits into from
Closed

Conversation

c0d3h4x0r
Copy link

joy2key improvements

  • joy2key.py: fix decode error (that occurred whenever return value was anything other than bytes)
  • joy2key.py: remove usage of os.fork() that was preventing shell scripts from using $! to get definitive pid
  • joy2key.py: combine disparate lists into a single set of class objects, so related state can no longer get out of sync
  • joy2key.py: create a _main() function to get things out of global scope
  • joy2key.py: rename global variables per coding conventions and move them near the top
  • joy2key.py: comment out the totally broken repeated event filtering (joystick control is now much more responsive)
  • joy2key.py: simplify timing and tighten up responsiveness by removing/shortening some now-unnecessary sleeps
  • joy2key.py: restructured and simplified the main loop for clarity and maintainability
  • helpers.sh, runcommand.sh: stop guessing at the joy2key.py pid using pgrep, and just definitively obtain it via $!
  • helpers.sh, runcommand.sh: fix functional inconsistencies (use same variable name, use same timings, etc)

  * joy2key.py: fix decode error (that occurred whenever return value was anything other than bytes)
  * joy2key.py: remove usage of os.fork() that was preventing shell scripts from using $! to get definitive pid
  * joy2key.py: combine disparate lists into a single set of class objects, so related state can no longer get out of sync
  * joy2key.py: create a _main() function to get things out of global scope
  * joy2key.py: rename global variables per coding conventions and move them near the top
  * joy2key.py: comment out the totally broken repeated event filtering (joystick control is now much more responsive)
  * joy2key.py: simplify timing and tighten up responsiveness by removing/shortening some now-unnecessary sleeps
  * joy2key.py: restructured and simplified the main loop for clarity and maintainability
  * helpers.sh, runcommand.sh: stop guessing at the joy2key.py pid using pgrep, and just definitively obtain it via $!
  * helpers.sh, runcommand.sh: fix functional inconsistencies (use same variable name, use same timings, etc)
@joolswills
Copy link
Member

Sorry - but I am going to have to turn this down for a few reasons.

There's a reason we used fork(). Check the source history. You have likely brought back that issue. We used to background via & - your code looks like it reverts to the old behaviour.

Your reorganisation of the python isn't where I want to go currently, sorry. But I am aware it needs work, and will note any of your improvements. Thanks.

Just to note also: You seem to have multiple commits with fixes for earlier commits. Also merge commits - these are not wanted in PRs in general. It's better to rebase your file changes into logical commits (and squash later fixes into previous changes as needed).

@joolswills joolswills closed this Jul 14, 2020
@c0d3h4x0r
Copy link
Author

And with that, I am done trying to contribute my talents back to RetroPie. My efforts are clearly neither desired nor appreciated, so I will stop wasting my time trying to help.

@c0d3h4x0r
Copy link
Author

Good luck with your bug farm.

@RetroPie RetroPie locked as too heated and limited conversation to collaborators Jul 14, 2020
@c0d3h4x0r c0d3h4x0r deleted the joy2key-improvements branch July 24, 2020 07:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants