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.

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

Add feature flag for cpu_clock_hz() #40

Merged
merged 5 commits into from
Nov 18, 2022

Conversation

johnathancn
Copy link
Contributor

No description provided.

@schteve
Copy link
Collaborator

schteve commented Oct 27, 2022

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
Copy link
Collaborator

schteve commented Oct 27, 2022

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 johnathancn changed the title conditionally compile freertos_rs_get_configCPU_CLOCK_HZ Add feature flag for cpu_clock_hz() Nov 17, 2022
@johnathancn
Copy link
Contributor Author

@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
Copy link
Collaborator

schteve commented Nov 18, 2022

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
Copy link
Contributor Author

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

@schteve
Copy link
Collaborator

schteve commented Nov 18, 2022

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

@schteve schteve merged commit 1ffc1a2 into lobaro:master Nov 18, 2022
@johnathancn johnathancn deleted the configCPU_CLOCK_HZ branch November 18, 2022 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants