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

Add mem{8,16,32} to machine. #35

Merged
merged 1 commit into from
May 4, 2022
Merged

Add mem{8,16,32} to machine. #35

merged 1 commit into from
May 4, 2022

Conversation

microbit-matt-hillsdon
Copy link
Contributor

@microbit-matt-hillsdon
Copy link
Contributor Author

@microbit-carlos, would be great if you could review. Is there some better option for the original code on the linked issue?

@microbit-matt-hillsdon
Copy link
Contributor Author

Preview build of the editor with these stubs building now. Will be at:
https://review-python-editor-next.microbit.org/mem/
microbit-foundation/python-editor-v3#724

@microbit-carlos
Copy link
Contributor

Is there some better option for the original code on the linked issue?

Not sure I understand the question, better than what code? Accessing random memory using machine.mem32?

@microbit-matt-hillsdon
Copy link
Contributor Author

Not sure I understand the question, better than what code? Accessing random memory using machine.mem32?

@microbit-carlos, from what I can read from the source in the screenshot it seems like it's not randomly chosen, but instead to set pin0 into high drive mode. Image is on microbit-foundation/python-editor-v3#723 (comment)

@microbit-carlos
Copy link
Contributor

Right, sorry, I didn't mean "random" in the literal sense. But I guess that does answer the question about what you were referring to when you said "original code".

That is the only way right now to configure peripherals in MicroPython to something different than what CODAL does (using Python code). So that's why they accessory library has done that.

Nevertheless, even if there was a better way to do that, this PR is still required to correctly parse the machine.mem8/16/32 instances.

@microbit-carlos
Copy link
Contributor

Looking at this image:
image

There is no way to hide the mem class definition in the API viewer?

@microbit-carlos
Copy link
Contributor

I guess is similar to MicroBitDigitalPin, which as classes that will appear in the API sidebar, but cannot be used in the code.
Is it worth adding a note that the class cannot be instantiated?

@microbit-matt-hillsdon
Copy link
Contributor Author

I guess is similar to MicroBitDigitalPin, which as classes that will appear in the API sidebar, but cannot be used in the code. Is it worth adding a note that the class cannot be instantiated?

I nearly did that, I expected to find text I could reuse on Button or MicroBitDigitalPin but we haven't done it there. I also felt that "instantiated" is a bit jargon-y. I'll raise a separate issue.

@microbit-matt-hillsdon
Copy link
Contributor Author

I nearly did that, I expected to find text I could reuse on Button or MicroBitDigitalPin but we haven't done it there. I also felt that "instantiated" is a bit jargon-y. I'll raise a separate issue.

Raised #37

@microbit-matt-hillsdon microbit-matt-hillsdon merged commit 13c9f69 into main May 4, 2022
@microbit-matt-hillsdon microbit-matt-hillsdon deleted the mem branch May 4, 2022 15:59
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.

2 participants