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

💻 Implemented "is" and sleep command for Micro:bit for level 2 #5587

Merged
merged 28 commits into from
Jun 18, 2024

Conversation

rmagedon97
Copy link
Collaborator

@rmagedon97 rmagedon97 commented Jun 3, 2024

Added the assignment to a variable from level 2 in Hedy to Micro:bit
Added sleep functionality for Micro:bit

How to test

Follow these steps to verify this PR works as intended:
Open Hedy and choose level 2. Assign a variable and then print it. This will generate a hex file that can be uploaded here:
https://python.microbit.org/v/3/reference.
You should see the correct python code after you upload the file.

  • Contains one of the PR categories in the name
  • Describes changes in the format above
  • Links to an existing issue or discussion
  • Has a "How to test" section

If you're unsure about any of these, don't hesitate to ask. We're here to help!

@rmagedon97 rmagedon97 requested a review from Felienne June 3, 2024 17:02
@rmagedon97 rmagedon97 changed the title 💻 Implemented "is" in Micro:bit for level 1 and 2 💻 Implemented "is" and sleep command for Micro:bit for level 2 Jun 3, 2024
hedy.py Outdated Show resolved Hide resolved
hedy.py Outdated Show resolved Hide resolved
hedy.py Outdated Show resolved Hide resolved
hedy.py Outdated Show resolved Hide resolved
hedy.py Outdated Show resolved Hide resolved
@boryanagoncharenko boryanagoncharenko marked this pull request as draft June 6, 2024 14:00
Copy link
Collaborator

@boryanagoncharenko boryanagoncharenko left a comment

Choose a reason for hiding this comment

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

As agreed during the meeting with Teodor:

  • I am converting both PRs to draft, since some changes are needed
  • All added whitespacing needs to be removed from the microbit transpiler and added where the program is put together with the while True: cycle.
  • Tests need to be added for the commands that are changed (the extent depends on the overall status of the project)

rmagedon97 and others added 8 commits June 7, 2024 09:07
Got rid of the white spaces in the transpiler. Modified the save_transpiled_code_for_microbit function to display microbit code properly.
The assign function now checks if the value is string it encapsulates it in ' ' if it is a number there will  be no ' '
@jpelay
Copy link
Member

jpelay commented Jun 11, 2024

While looking at your code I see that you generate a file in the backend and then send it to the frontend, where it downloads. What would happen if there are concurrent calls to the endpoint? I'm pretty sure that will cause either slowdowns or race conditions. I think that a much better approach would be to send the transpiled code to the front-end and generate the file directly in the user's machine, this will prevent having to write to disc for every call.

@Felienne
Copy link
Member

While looking at your code I see that you generate a file in the backend and then send it to the frontend, where it downloads. What would happen if there are concurrent calls to the endpoint? I'm pretty sure that will cause either slowdowns or race conditions. I think that a much better approach would be to send the transpiled code to the front-end and generate the file directly in the user's machine, this will prevent having to write to disc for every call.

Do we do that for dst files too (if not, that is probably a wise idea!) Maybe we can combine this into one part of the code that does that?

@jpelay
Copy link
Member

jpelay commented Jun 11, 2024

Do we do that for dst files too (if not, that is probably a wise idea!) Maybe we can combine this into one part of the code that does that?

I just checked and for those files we generate a unique random filename, this would also prevent race conditions in this case!

hedy.py Outdated Show resolved Hide resolved
hedy.py Outdated Show resolved Hide resolved
tests/test_level/test_level_02.py Outdated Show resolved Hide resolved
hedy.py Show resolved Hide resolved
@rmagedon97
Copy link
Collaborator Author

While looking at your code I see that you generate a file in the backend and then send it to the frontend, where it downloads. What would happen if there are concurrent calls to the endpoint? I'm pretty sure that will cause either slowdowns or race conditions. I think that a much better approach would be to send the transpiled code to the front-end and generate the file directly in the user's machine, this will prevent having to write to disc for every call.

That's a good idea. I basically did the same was as we are doing the files for the turtle. I had some issues converting the file to hex that's why i used he same name for the file. After your comment i'll try to use a random file name.

@rmagedon97 rmagedon97 marked this pull request as ready for review June 17, 2024 00:04
Copy link
Collaborator Author

@rmagedon97 rmagedon97 left a comment

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

mergify bot commented Jun 18, 2024

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit dd65e32 into main Jun 18, 2024
12 checks passed
@mergify mergify bot deleted the Microbit-Level_1 branch June 18, 2024 20:01
Copy link
Contributor

mergify bot commented Jun 18, 2024

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants