Skip to content
This repository has been archived by the owner on Dec 23, 2021. It is now read-only.

Adding debugger communication #113

Merged
merged 13 commits into from
Aug 15, 2019
Merged

Conversation

Christellah
Copy link
Contributor

@Christellah Christellah commented Aug 13, 2019

Description:

This PR is for adding a socket communication between our extension (TS server) and the Python process (client) when the user starts a debug session.

How it's done :

  • The general idea is that in debugging mode, instead of using a process's stdin/out to communicate, we exchange messages via Sockets using Socket.io (https://socket.io/)
  • We catch the event onDebugSessionStarted in extension.ts and we launch a socket server CommunicationHandlerServer.ts.
  • The debug configuration is set up to run debud_user_code.py instead of the regular process_user_code.py.
  • debud_user_code.py will start a Python client CommununicationHandlerClient.py and set our API's inDebugMode boolean to true.
  • In our API, when we call the show() method, we check if we're in debug mode, if yes we use socket communication, otherwise we use the old method with print statements.

Note:

  • This PR fixes a bug:32634 that made the play button in the webview not change state accordingly. So now we're waiting for a confirmation from the extension that the code is running before changing the state of this button.
  • The current behavior is : if the code is running and you try to debug, it will stop running and start debugging. If you're debugging, it will prevent you from running.

Remember: You need to pip install python-socketio and pip install requests and then run npm i

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Limitations:

  • The port is encoded for now. The next PR will let the user change it.
  • We're not checking if the port is in use for now, it will be addressed in the next PR.
  • We use two different types of communication for the regular way and for debugging.

Testing:

  • Try to debug the code (add breakpoints, run line by line...) make sure the code is still executing correctly and you can see the simulator being updated
  • Try to run it the usual way and make sure it's still working

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

src/communicationHandlerServer.ts Outdated Show resolved Hide resolved
src/communicationHandlerServer.ts Outdated Show resolved Hide resolved
src/debug_user_code.py Outdated Show resolved Hide resolved
src/communicationHandlerServer.ts Outdated Show resolved Hide resolved
src/debug_user_code.py Outdated Show resolved Hide resolved
previousState = copy.deepcopy(state)
time.sleep(TIME_DELAY)
if debug_mode:
debugger_communication_client.update_state(json.dumps(message))
Copy link

Choose a reason for hiding this comment

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

Do we not need to sleep in debug mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't because sockets are efficient enough to handle a lot of messages (whereas the pipe of the process gets fulled), at least so far I didn't encounter any issue with the auto-write functionality

Copy link
Member

@LukeSlev LukeSlev left a comment

Choose a reason for hiding this comment

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

It works ✔ one thing I noticed tho, we always run our code even if someone uses a different config... should fix that somewhere

src/adafruit_circuitplayground/express.py Show resolved Hide resolved
src/constants.ts Show resolved Hide resolved
src/debug_user_code.py Outdated Show resolved Hide resolved
src/debug_user_code.py Show resolved Hide resolved
src/debuggerCommunicationServer.ts Show resolved Hide resolved
@Christellah Christellah merged commit 5cc4658 into dev Aug 15, 2019
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.

4 participants