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

Clue Debugger #294

Merged
merged 23 commits into from
Apr 8, 2020
Merged

Clue Debugger #294

merged 23 commits into from
Apr 8, 2020

Conversation

andreamah
Copy link
Contributor

@andreamah andreamah commented Apr 2, 2020

Description:

BACKEND:

Fixes:

Made debug_mode a field in the common lib rather than just instantiated separately in each microcontroller model. Then, in order to access it, you just need to import utils.

New:

Debugging for screen and buttons

Frontend changes:

Using the debugger adapter, we will freeze the UI to follow proper debugging behaviour since the board cannot read inputs while on PAUSE state.

-The buttons will not be clickable
-The sensors will not send new values
-Gesture button will not send anything

Type of change

@xnkevinnguyen xnkevinnguyen changed the title WIP: Clue Debugger Clue Debugger Apr 8, 2020
Copy link

@nasadigital nasadigital left a comment

Choose a reason for hiding this comment

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

Great job on making this work 🌻 🌻 🌻
LGTM!

Comment on lines +29 to +34
# Insert absolute path to library for CLUE into sys.path
sys.path.insert(0, os.path.join(abs_path_to_parent_dir, CONSTANTS.CLUE))

# Insert absolute path to Circuitpython libraries for CLUE into sys.path
sys.path.insert(0, os.path.join(abs_path_to_parent_dir, CONSTANTS.CIRCUITPYTHON))

Choose a reason for hiding this comment

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

Why do we need to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you run tests, it doesn't run process_user_code.py, which is where this usually happens. This adds src/clue and src/micropython to the sys path so that we can import microbit and clue.

We need to put it in debug_user_code.py rather than its test because some tests import debug_user_code and need these sys.path add-ons in order to import the file.

Choose a reason for hiding this comment

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

Thanks for clarifying!
And why do we need this in process_user_code.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code automatically looks for packages in the out/* directory, but doesn't recursively search. This is so that it can find packages that don't necessarily sit right below out/*. In the case of microbit and clue, they both have their packages defined in sub-folders.

Choose a reason for hiding this comment

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

Which modules do we want to use? Why can't use the import statement to do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for the case of process_user_code.py, we wanted the user to import the packages using a specific way, so we're doing it this way. For all of the imports, we want to keep the method consistent because import micropython.microbit and import microbit imports two different instances of the microbit package.

Choose a reason for hiding this comment

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

Sorry, I am struggling to understand what we are trying to achieve here.
If import micropython.microbit and import microbit import two different instances, why don't we use one or the other? In which case do we want to use one over the other?

@xnkevinnguyen xnkevinnguyen merged commit 0df6274 into dev Apr 8, 2020
@xnkevinnguyen xnkevinnguyen deleted the users/t-anmah/clue-debugger branch April 9, 2020 16:45
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.

3 participants