lobaro / FreeRTOS-rust

Rust crate for FreeRTOS
MIT License
326 stars 55 forks source link

Add feature flag for cpu_clock_hz() #40

Closed johnathancn closed 1 year ago

schteve commented 1 year ago

So, for me, this change works correctly for the Linux example but not for the Windows one. I get a linker error like this: error LNK2019: unresolved external symbol freertos_rs_get_configCPU_CLOCK_HZ referenced in function _ZN13freertos_rust5utils12cpu_clock_hz17h2f37b96a23ba4a09E I don't get any error for linux.

I don't know why this happens - neither example defines configCPU_CLOCK_HZ and cpu_clock_hz() isn't used anywhere. My only guess is that the linker on Linux is smarter about eliminating dead code in this situation.

For what it's worth, the error I get with the #ifdef isn't nearly as clear to the user as what I get now by default: error C2065: 'configCPU_CLOCK_HZ': undeclared identifier

schteve commented 1 year ago

As additional data points, building with -C link-dead-code has no effect on the errors, but building the win example in release mode does succeed.

johnathancn commented 1 year ago

@schteve I have gone ahead with the feature flag idea (updated PR title). See updated branch. For me I was able to build Linux, Windows, and black pill demos. For black pill I added a conditional demo call to cpu_clock_hz() to build the feature.

I noticed I could never build Cortex-M3 and nRF9160 demos. They both fail with "rust-lld: error:"... "cannot find linker script device.x". I see this in both my build environments (windows and Linux). Any idea about this?

schteve commented 1 year ago

In the interest of moving this along, I say we go ahead with the feature flag idea (since I didn't get anywhere with the linker error so still not sure if is a viable route). We can deal with the broader project implications in a separate PR when (if) we get there. For now merging this PR will at least get things building and running again.

They both fail with "rust-lld: error:"... "cannot find linker script device.x".

I get the same errors, and I'm not sure when these examples last build correctly or what broke them. I think device.x is usually supplied by one of the other crates in the embedded stack like a PAC. For now, since it's already broken, we can ignore it here.

I'll add a few quick comments on the code but otherwise think we can merge it if there's nothing else to change.

johnathancn commented 1 year ago

@schteve thanks for your comments, please see all addressed.

schteve commented 1 year ago

Thanks for your work and your patience as we tried out a few options!