jonas-schievink / rubble

(going to be a) BLE stack for embedded Rust
BSD Zero Clause License
396 stars 56 forks source link

Update dependencies #100

Closed fmckeogh closed 4 years ago

fmckeogh commented 4 years ago

Updates dependencies project-wide, the only non patch bump is a minor bump in nrf51-hal. Does the bbqueue v0.4.0 release mean we can move away from a git dependency for it? James said thumbv6 support was implemented, but I've forgotten if there was something else that was blocking.

jonas-schievink commented 4 years ago

Does the bbqueue v0.4.0 release mean we can move away from a git dependency for it?

Yes, it should. It's currently only used for the logging buffer in the demo, so thumbv6 support isn't critical (it does hopefully allow making the demo work on the nRF51 though).

jonas-schievink commented 4 years ago

Oh, and the nRF51 update should probably wait until we've released the new nRF52 HALs and PACs, or we'd have to deal with PACs generated by different versions of svd2rust.

fmckeogh commented 4 years ago

Done and done :)

Edit: hold on, it was a breaking change and some lifetimes need to be added.

fmckeogh commented 4 years ago

Before/after bbqueue version bump:

~/D/r/d/nrf52-demo ((c5335333…)|✚1) $ cargo size --bin nrf52-demo
   Compiling nrf52-demo v0.0.0 (/Users/ferdiamckeogh/Downloads/rubble/demos/nrf52-demo)
    Finished dev [optimized + debuginfo] target(s) in 0.81s
   text    data     bss     dec     hex filename
  84792      20   10372   95184   173d0 nrf52-demo
~/D/r/d/nrf52-demo (update_deps|✚1) $ cargo size --bin nrf52-demo
   Compiling nrf52-demo v0.0.0 (/Users/ferdiamckeogh/Downloads/rubble/demos/nrf52-demo)
    Finished dev [optimized + debuginfo] target(s) in 0.73s
   text    data     bss     dec     hex filename
  83900   10040     340   94280   17048 nrf52-demo

I also changed the BbqLogger doc comment to reflect the bbqueue rename, it's entirely up to you if you want this. I suppose it might also make sense to rename BbqLogger to BbbLogger or something else, I guess?

Either way it's all working and tested on my two nRF52s :)

fmckeogh commented 4 years ago

I can also split the two commits into two PRs if you'd prefer :)

jonas-schievink commented 4 years ago

Nice! But the backing store seems to have moved from bss to data, increasing flash usage a lot. Is this something we have to fix here or is that a bbqueue bug?

fmckeogh commented 4 years ago

I think it's because of this change, from this to this, where now each half needs a reference to the created BBBuffer so we can't return a reference to the created BBBuffer here. To fix this we could either repeat the static Option pattern used for Logger, or make a const buffer, so I chose the latter. I can change it to the former if you want, unless there is an easier way?

fmckeogh commented 4 years ago

Ah wait, there's also the problem of when the log feature isn't enabled, I'll need to fix that too.

jonas-schievink commented 4 years ago

I think that change is due to the switch in bbqueue from zero-initializing the buffer here to using MaybeUninit::uninit() here. I think this might be a known issue with rustc, I'll check.

jamesmunns commented 4 years ago

On the all-bss branch of bbqueue, the "uses too much .data" seems to be fixed:

james@archx1c6g ➜  nrf52-demo git:(update_deps) ✗ cargo build --release --features 52832 && arm-none-eabi-size ../../target/thumbv7em-none-eabi/release/nrf52-demo
    Updating crates.io index
   Compiling nrf52-demo v0.0.0 (/tmp/rubble/demos/nrf52-demo)
    Finished release [optimized + debuginfo] target(s) in 4.20s
   text    data     bss     dec     hex filename
  43076   10040     340   53456    d0d0 ../../target/thumbv7em-none-eabi/release/nrf52-demo

# Switch to a patched version of bbqueue

james@archx1c6g ➜  nrf52-demo git:(update_deps) ✗ cargo build --release --features 52832 && arm-none-eabi-size ../../target/thumbv7em-none-eabi/release/nrf52-demo
    Updating git repository `https://github.com/jamesmunns/bbqueue.git`
    Finished release [optimized + debuginfo] target(s) in 0.51s
   text    data     bss     dec     hex filename
  43072      20   10360   53452    d0cc ../../target/thumbv7em-none-eabi/release/nrf52-demo

patch:

diff --git a/Cargo.toml b/Cargo.toml
index 33321c7..3954f6c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -14,3 +14,7 @@ opt-level = "s"
 debug = true
 lto = true
 codegen-units = 1
+
+[patch.crates-io.bbqueue]
+git = "https://github.com/jamesmunns/bbqueue.git"
+rev = "all-bss"
jamesmunns commented 4 years ago

bbqueue v0.4.1 has been released

jonas-schievink commented 4 years ago

@jamesmunns Awesome, thanks for the quick fix!

fmckeogh commented 4 years ago

Bumped version and tested :)