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.

Sign up for GitHub

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

Jump to bottom

Zephyr rebase #2

Merged
merged 5 commits into from
Oct 9, 2016
Merged

Zephyr rebase #2

merged 5 commits into from
Oct 9, 2016

Conversation

daniel-thompson
Copy link
Contributor

Hi Paul

Rebasing on your 'connect branch turned out to be a nop so I thought I'd send in a PR anyway. If you've already started rebasing your stuff then no worries. Just ping when you're done and I can rebase again...

Daniel.

@daniel-thompson
zephyr: Exploit "make outputexports"
9ef2ec9 
The outputexpors target, which exports Zephyr environment variables, was
recently added to Zephyr. By exploiting this feature we can hugely simplify
the build system, improving robustness at the same time.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
@daniel-thompson
zephyr: Allow NETWORKING and MBEDTLS to be disabled
eddd31f 
Allow micropython to automatically disable features that
Zephyr has been configured not to support.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
@daniel-thompson
zephyr: Implement the help() function
bba5d67 
The boot issue text mentions a help() function and encourages
the user to run it. It is very disconcerting to find that the
function does not exist...

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
@daniel-thompson
zephyr: Implement utime module
29eccb1 
This provides time and sleep together with the usual
ticks_us/_ms_diff and sleep_us/ms family.

We also provide access to Zephyr's high precision timer
as ticks_cycles (augmented by cycles_to_ns).

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
@daniel-thompson
zephyr: Initial attempt at machine.Pin
676e97c 
The integration with Zephyr is fairly clean but requires us to use
two arguments to describe a pin:

  drv_name - Name to be sent to device_get_binding()
  pin      - Pin number within the port identified by drv_name

There is support for in/out pins and pull up/pull down but currently
there is no interrupt support.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
@daniel-thompson daniel-thompson changed the base branch from master to zephyr October 9, 2016 20:20
@pfalcon pfalcon merged commit 676e97c into pfalcon:zephyr Oct 9, 2016
@pfalcon
Copy link
Owner

pfalcon commented Oct 9, 2016

Thanks Daniel. I didn't proceed with rebasing/merging so far, as I lost comfortable access to github for now. But as this PR cleanly applies on top of my zephyr branch, let me just merge it into it, and tomorrow I plan to squash/merge stuff. Thanks!

@pfalcon
Copy link
Owner

pfalcon commented Oct 10, 2016 •
edited
Loading

Ok, cherry-picked into the master the Makefile evolution up to and including "Exploit "make outputexports"", while squashing and cleaning it up whenever possible (while squashing the main aim was preserving authorship of your patches, thought I squashed couple of your smaller changes). Can you please verify that "Exploit "make outputexports"" was merged faithfully? (There were quite a bunch of conflicting resolution cases due to cherry-picking build system changes out of complete change arrays).

@daniel-thompson
Copy link
Contributor Author

Looks OK to me.

Note that the remains of the 'Exploit "make outputexports"' patch no longer really makes sense on its own. I'd suggest splitting it out into the three changes (modusocket.c, .gitignore, README.md) and squashing these into your own patches. The same goes for "zephyr: Allow NETWORKING and MBEDTLS to be disabled"; this is better squashed into the socket/mbedtls code.

For the build system I agree trying to maintain authorship just makes the patchset look wierd. Rather than trying to maintain authorship, its probably better just to put a credit in the patch description of one of the squashed patches.

@pfalcon
Copy link
Owner

pfalcon commented Oct 10, 2016

Looks OK to me.

Thanks for confirming that!

this is better squashed into the socket/mbedtls code.

Will do. I plan to write a new README from scratch. And I don't plan to merge modsocket, given that it barely works, and should be rewritten against new Z IP stack (which itself needs work first).

For the build system I agree trying to maintain authorship just makes the patchset look wierd.

Well, I think it's ok, and it makes sense to preserve the evolution of Zephyr build system integration, to be a live evidence why "outputexports" was needed.

Ok, so I'll proceed with merging few other low-hanging things, to fit into micropython 1.8.5 release. Other things (e.g. machine.Pin) may need more discussion/work.

@daniel-thompson
Copy link
Contributor Author

Sure. Getting the basic port upstream first is the most important step. Once that's landed I can send PRs to the main project like any other developer!

On October 10, 2016 4:23:08 PM GMT+01:00, Paul Sokolovsky notifications@github.com wrote:

Looks OK to me.

Thanks for confirming that!

this is better squashed into the socket/mbedtls code.

Will do. I plan to write a new README from scratch. And I don't plan to
merge modsocket, given that it barely works, and should be rewritten
against new Z IP stack (which itself needs work first).

For the build system I agree trying to maintain authorship just makes
the patchset look wierd.

Well, I think it's ok, and it makes sense to preserve the evolution of
Zephyr build system integration, to be a live evidence why
"outputexports" was needed.

Ok, so I'll proceed with merging few other low-hanging things, to fit
into micropython 1.8.5 release. Other things (e.g. machine.Pin) may
need more discussion/work.

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#2 (comment)

Sent using a tiny, tiny keyboard. Pleasw excise any spelkinh nistajes.

@pfalcon
Copy link
Owner

pfalcon commented Oct 12, 2016

@daniel-thompson : I'm going to merge "Implement the help() function" to master due to reasons you mention in the commit, but help() implementation is a major source of code duplication in uPy, and it's on TODO for refactoring...

P.S. It seems that your Linaro commit email isn't recognized by github as belonging to your account, which causes minor usability issues (like inability to @ mention easily), could you please consider adding it to your github account?

@pfalcon
Copy link
Owner

pfalcon commented Oct 12, 2016

Otherwise, I think master now contains pretty good basic Zephyr port, just in time for 1.8.5. So, let the release happen and then look into further things.

@daniel-thompson
Copy link
Contributor Author

Yes. When I wrote it I wondered why it didn't walk the list of built in modules to automate much of the help...

PS Done. My work mail is now hooked up to github.

On October 12, 2016 5:25:41 PM GMT+01:00, Paul Sokolovsky notifications@github.com wrote:

@daniel-thompson : I'm going to merge "Implement the help() function"
to master due to reasons you mention in the commit, but help()
implementation is a major source of code duplication in uPy, and it's
on TODO for refactoring...

P.S. It seems that your Linaro commit email isn't recognized by github
as belonging to your account, which causes minor usability issues (like
inability to @ mention easily), could you please consider adding it to
your github account?

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#2 (comment)

Sent using a tiny, tiny keyboard. Pleasw excise any spelkinh nistajes.

@pfalcon
Copy link
Owner

pfalcon commented Oct 13, 2016

@daniel-thompson : So, I had a look at your modutime.c, and here're comments:

  1. Please start with implementing mp_hal_ticks_us() and friends. These would be needed to enjoy other reusable MicroPython components, like I2C/SPI bitbanging implementation.
  2. ticks_cycles() should be ticks_cpu(), as uPu API spec calls. It's defined as returning highest-frequency timer value (exact frequency is underspecified).
  3. There's no cycles_to_ns() in utime module API. Feel free to argue for need for it, but my likely response then would be that it should go to adhoc "zephyr" module. Otherwise, we'll have API creep and never will be able to write portable apps.

Thanks.

@pfalcon
Copy link
Owner

pfalcon commented Oct 13, 2016

Please start with implementing mp_hal_ticks_us() and friends. These would be needed to enjoy other reusable MicroPython components, like I2C/SPI bitbanging implementation.

And here's modutime deduplication refactor I meant previously: micropython/micropython#2514

pfalcon pushed a commit that referenced this pull request Dec 2, 2021
@jepler @pfalcon
py/repl: Don't read past the end of import_str.
a8c8213 
asan considers that memcmp(p, q, N) is permitted to access N bytes at each
of p and q, even for values of p and q that have a difference earlier.
Accessing additional values is frequently done in practice, reading 4 or
more bytes from each input at a time for efficiency, so when completing
"non_exist<TAB>" in the repl, this causes a diagnostic:

    ==16938==ERROR: AddressSanitizer: global-buffer-overflow on
    address 0x555555cd8dc8 at pc 0x7ffff726457b bp 0x7fffffffda20 sp 0x7fff
    READ of size 9 at 0x555555cd8dc8 thread T0
        #0 0x7ffff726457a  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
        #1 0x555555b0e82a in mp_repl_autocomplete ../../py/repl.c:301
        #2 0x555555c89585 in readline_process_char ../../lib/mp-readline/re
        #3 0x555555c8ac6e in readline ../../lib/mp-readline/readline.c:513
        #4 0x555555b8dcbd in do_repl /home/jepler/src/micropython/ports/uni
        #5 0x555555b90859 in main_ /home/jepler/src/micropython/ports/unix/
        #6 0x555555b90a3a in main /home/jepler/src/micropython/ports/unix/m
        #7 0x7ffff619a09a in __libc_start_main ../csu/libc-start.c:308
        #8 0x55555595fd69 in _start (/home/jepler/src/micropython/ports/uni

    0x555555cd8dc8 is located 0 bytes to the right of global variable
    'import_str' defined in '../../py/repl.c:285:23' (0x555555cd8dc0) of
    size 8
      'import_str' is ascii string 'import '

Signed-off-by: Jeff Epler <jepler@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@daniel-thompson @pfalcon