ntoll / uflash

A module and command to easily flash Python onto the BBC's micro:bit device.
http://micropython.org/
MIT License
101 stars 27 forks source link

Make consistent with mu editor version - this version causes an AttributeError exception #43

Closed MrYsLab closed 6 years ago

MrYsLab commented 6 years ago

I have a very simple program:

from microbit import *

pin0.set_pull(pin0.PULL_UP)

Flashing from mu causes no errors, but flashing from the code in this distribution, I get:

  File "__main__", line 3, in <module>
AttributeError: 'MicroBitTouchPin' object has no attribute 'set_pull'

Both uflash.py in this repo and in mu contain the same version number, yet when I do a file compare, I get the following differences:

From mu:

def hexlify(script):
    """
    Takes the byte content of a Python script and returns a hex encoded
    version of it.

    Based on the hexlify script in the microbit-micropython repository.
    """
    if not script:
        return ''
    # Convert line endings in case the file was created on Windows.
    script = script.replace(b'\r\n', b'\n')
    script = script.replace(b'\r', b'\n')
    # Add header, pad to multiple of 16 bytes.
    data = b'MP' + struct.pack('<H', len(script)) + script
    # Padding with null bytes in a 2/3 compatible way
    data = data + (b'\x00' * (16 - len(data) % 16))
   # -----> different from below
    assert len(data) <= 0x2000

From this repo:

def hexlify(script):
    """
    Takes the byte content of a Python script and returns a hex encoded
    version of it.

    Based on the hexlify script in the microbit-micropython repository.
    """
    if not script:
        return ''
    # Convert line endings in case the file was created on Windows.
    script = script.replace(b'\r\n', b'\n')
    script = script.replace(b'\r', b'\n')
    # Add header, pad to multiple of 16 bytes.
    data = b'MP' + struct.pack('<H', len(script)) + script
    # Padding with null bytes in a 2/3 compatible way
    data = data + (b'\x00' * (16 - len(data) % 16))
   # ------> different from above
    if len(data) > 8192:
        # 'MP' = 2 bytes, script length is another 2 bytes.
        raise ValueError("Python script must be less than 8188 bytes.")

Please note that I did not change the version number, which will keep it consistent with the mu version version number.

carlosperate commented 6 years ago

Thanks for bringing this up @MrYsLab.

This change would be a regression, as the assert was replaced by the raising the exception in https://github.com/ntoll/uflash/commit/757201393d2101a9baa88044e6ddddf365dd32ff.

It's possible the version number is the same because it is only increased when ready for a new PyPi release, and there hasn't been one yet. So, v1.0.8 would be the state of master at https://github.com/ntoll/uflash/commit/065aca4dfdd9824e30b62cfe2cbf4eb14d5b5c65.

Apart from that the version bundled in Mu has been manually edited to include the latest version of MicroPython (v1.0-beta), but this is only for the beta release of Mu. Once MicroPython v1.0 is released, uFlash should be upreved and released, and that version then properly bundled with Mu.

MrYsLab commented 6 years ago

I now understand that this would be a regressive change, none the less, there is a bug in the current PyPi version of uflash. The example I gave, a 2 line program is not throwing an exception on the host PC but a runtime exception on the micro:bit. It is not exceeding any coding limit. I am working on a Python library for the Sparkfun gamer:bit that requires that some of the pins need to have their pull_up resistors enabled. I have modified my local copy of the uflash library to use the current mu version of uflash.py, but when I release, I will need to add a note that users should use mu instead of PyCharm (which uses the PyPi version of uflash) or a simple text editor.

I would like to make sure that this bug does not get lost, so should I enter an issue for this and close the pull request?

Also, if at all possible, please update the version numbers, even for alpha or beta versions, so that there are not 2 different versions of software sporting the same version number.

Thanks for getting back to me so quickly on this.

carlosperate commented 6 years ago

Sorry, I should have clarified that the exception you see on the micro:bit is because the set_pull method is not implemented in version 0.9 of MicroPython, so you'll have to use version v1.0-beta instead. The reason it works with Mu's version of uFlash is because MicroPython 1.0-beta was manually added in that repository, to be able to test it in workshops.

You can specify a MicroPython hex file as another parameter for uFlash, the instructions for that should be present in the README.

MrYsLab commented 6 years ago

I will close this pull request. It sounds like when 1.0 is released this will all go away, so I will not muddy the waters with entering an issue unless you would like me to.