esp-rs / esp32-hal

A hardware abstraction layer for the esp32 written in Rust.
Apache License 2.0
192 stars 28 forks source link

Crates.io release #46

Closed MabezDev closed 3 years ago

MabezDev commented 3 years ago

I think the functionality of this crate is in a pretty good place (mainly thanks to @arjanmels hard work!).

Anything we're missing? Docs, examples?

I need to update the xtensa quickstart, ideally with a minimal examples plus esp32 & esp8266 examples too, but thats unrelated.

arjanmels commented 3 years ago

I agree, that it has sufficient body for an initial release. (There is always more that can be done of course.)

Examples cover most of the hal except for the efuse module. Examples can use some cleanup here and there (e.g. to use timer module, instead of bit manipulation to disable watchdog timers). (I was also considering pulling common code in a shared file for the examples.)

Basic documentation is in place for most, could use a review for missing items (e.g. efuse module description, get_core function etc).

Question is if you want to do this all before a release on crates.io or if you want to do occasional updates as we go.

arjanmels commented 3 years ago

@MabezDev I just created a new pull request PR #47 with some updates I had pending (related to the work in progress on the WiFi), they do form some nice cleanups, you may want to consider merging before publishing, but it is not a must have.

MabezDev commented 3 years ago

@arjanmels I've been rewriting the xtensa-quickstart (see this pr).

Ideally I'd like to switch to using the new build-std feature of cargo, removing the need for xargo/xbuild. It's actually working great except for one snag, there is no way to control the "mem" feature of compiler builtins, meaning the "mem" feature of this crate doesn't work (complains about duplicate symbols). Essentially this issue: https://github.com/rust-lang/wg-cargo-std-aware/issues/53 but the inverse.

We can work around this by dumping the target json

$ $RUSTC -Z unstable-options --print target-spec-json --target=xtensa-esp32-none-elf

and adding no-builtins: true. The final json looking like this:

{
    "arch": "xtensa",
    "cpu": "esp32",
    "data-layout": "e-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32",
    "emit-debug-gdb-scripts": false,
    "env": "",
    "executables": true,
    "is-builtin": true,
    "linker": "xtensa-esp32-elf-gcc",
    "linker-flavor": "gcc",
    "llvm-target": "xtensa-none-elf",
    "max-atomic-width": 32,
    "os": "none",
    "panic-strategy": "abort",
    "relocation-model": "static",
    "target-c-int-width": "32",
    "target-endian": "little",
    "target-pointer-width": "32",
    "no-builtins": true,
    "unsupported-abis": [
        "stdcall",
        "fastcall",
        "vectorcall",
        "thiscall",
        "win64",
        "sysv64"
    ],
    "vendor": ""
}

It sucks to lose the performance gain from the custom mem routines, but I am thinking we should make them opt in for ease of transitioning to build-std. If someone is desperate for the performance, they could use xbuild and opt in to the feature.

What do you think?

arjanmels commented 3 years ago

It is not (only) about performance. More importantly it is about memory alignment: the instruction code SRAM can only be accessed by aligned instructions. The default routines do not guarantee this.

MabezDev commented 3 years ago

Ah if its required for functionality, we should definitely keep it enabled.

In which case I am leaning towards "no-builtins": true, set in the default configuration. Doing so allows us to use the custom mem routines, and anyone else looking to build for there own xtensa target must supply there own mem functions.

I am also thinking about the libc mem functions, seeing as we are using the C toolchain for linking we could probably link against the presumably already tailored mem functions for the esp.

MabezDev commented 3 years ago

I decided build-std isn't quite fit for use just yet and were back to using xbuild.

I have just published esp32-hal 0.1.0 to crates.io though! :tada: :tada: :tada: