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

Check and warn if mcu frequency differs from configured frequency #6519

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

KevinOConnor
Copy link
Collaborator

Several micro-controllers require a clock speed (or clock reference) to be configured in "make menuconfig". Normally if this value is not configured correctly then the host will be unable to communicate with the micro-controller. However, on a few boards, it is possible to establish communication with the mcu even if it has been incorrectly configured. Unfortunately, when this happens the connection is often unstable, unexpected errors can occur, or confusing printer motion can result.

This PR updates the host code to check if the detected mcu frequency is notably different from the configured mcu frequency. If there is a large difference then it will report a warning in the log and via the printer.configfile.warnings API server object.

@pedrolamas , @meteyou , @Arksine - as part of this change I have added a new type to the printer.configfile.warnings status object. The format is {"type": "runtime_warning", "message": "some message"}. I'm hoping this will not cause an issue for the frontends. If there is a concern, I can change the format and/or delay the rollout of that part of the change. I'm not sure printer.configfile.warnings is the best place to put this info, but it does seem to be a convenient place to report it.

-Kevin

@KevinOConnor KevinOConnor changed the title Check and warn if mcu frequency is differs from configured frequency Check and warn if mcu frequency differs from configured frequency Mar 6, 2024
@meteyou
Copy link
Contributor

meteyou commented Mar 6, 2024

Thx for the ping! I checked the code in Mainsail. It would ignore the new type and display it as a "danger warning" with your text. So so far it would not cause any problems in Mainsail.

Is there any way to test this warning? Or do you know a MCU witch can be set wrong? Maybe I have one at home to double-check it, and I want to add a link to the docs with a more detailed description.

@Sineos
Copy link
Collaborator

Sineos commented Mar 6, 2024

Maybe LPC1768 / LPC1769 or a HC32F460 board

@pedrolamas
Copy link
Contributor

In Fluidd we just display as a generic warning (mocked data, but proves the point):

image

@meteyou
Copy link
Contributor

meteyou commented Mar 6, 2024

Maybe LPC1768 / LPC1769 or a HC32F460 board

Thx @Sineos! I have some spare SKR 1.3 with LPC1768 at home. So, I should be able to reproduce this warning.

@pedrolamas
Copy link
Contributor

I tested the changes on this PR in my Docker simulavr container and... I guess now I know why simulavr is soooo slow!

image

Unrelated, but this also makes me wonder if there is anything we can do to improve the simulavr perf... 🤔

@Sineos
Copy link
Collaborator

Sineos commented Mar 6, 2024

I have some spare SKR 1.3 with LPC1768 at home.

Not sure if overclocking is going to work at all. Likely running a LPC1769 at 100MHz would be a suitable test candidate.

One recent example was the HC32F460 with a nominal frequency of 200MHz but compiled for 168MHz. It seemingly works but will produce strange errors during runtime.

@KevinOConnor
Copy link
Collaborator Author

Thanks.

Is there any way to test this warning?

I did some simple tests by changing the mcu.py code that says if mcu_freq_mhz != calc_freq_mhz to if mcu_freq_mhz == calc_freq_mhz. (That is, force the warning even when the two values are equal.) If changing the code be sure to run a full sudo service klipper restart.

-Kevin

@pedrolamas
Copy link
Contributor

Even if I prepare simulavr to run at 16MHz, I still get the 2MHz warning...

image

@KevinOConnor just double-checking, but is this just simulavr being weird?

@KevinOConnor
Copy link
Collaborator Author

is this just simulavr being weird?

How did you run simulavr? If you ran it from the command-line and told it to run at 2Mhz, then it would make sense that you are getting a report about 16Mhz (the "make menuconfig" setting) being different from 2Mhz (the command-line setting). Maybe I'm missing something though.

-Kevin

@pedrolamas
Copy link
Contributor

pedrolamas commented Mar 6, 2024

How did you run simulavr? If you ran it from the command-line and told it to run at 2Mhz, then it would make sense that you are getting a report about 16Mhz (the "make menuconfig" setting) being different from 2Mhz (the command-line setting)

This is the .config I used to build (set to 20MHz via make menuconfig) and this is the command line I used to start it (which has -s 20000000)

That's when I get this:

image


I also tried make menuconfig with 16MHz (matching the 2nd screenshot) and then just started with ./klipper/scripts/avrsim.py ./klipper/simulavr.elf

image


It seems simulavr is just ignoring the speed I set and runs at 2MHz?

@KevinOConnor
Copy link
Collaborator Author

KevinOConnor commented Mar 6, 2024

and this is the command line I used to start it

Oh, if you didn't specify a rate limiting then the avrsim.py tool will try to emulate instructions as fast as it can. I'd guess that means your CPU is achieving around 2Mhz of emulation per second. (The report in the warning is rounded to the nearest Mhz, but you can look at the "Stats" line in the logs to find the actual emulation rate.)

You can try adding something like -r 0.05 to set the emulation rate to 1Mhz. That will reduce the emulation speed, make the emulation speed more predictable, and reduce CPU usage. You can't, of course, make the emulation go faster if the CPU is maxing out at around 2Mhz though.

-Kevin

@meteyou
Copy link
Contributor

meteyou commented Mar 6, 2024

I have also just noticed that my mac mini m3 pro with virtual klipper printer, shows 3Mhz as mcu frequency. I used the "default settings" when creating simulavr firmware. so this would be possible, as Kevin explains, that this is just the "maximum emulation speed":

image

so it also looks to me, that ARM processors are better for emulating MCUs.

@pedrolamas
Copy link
Contributor

You can try adding something like -r 0.05 to set the emulation rate to 1Mhz. That will reduce the emulation speed, make the emulation speed more predictable, and reduce CPU usage. You can't, of course, make the emulation go faster if the CPU is maxing out at around 2Mhz though.

I will give it a try with -r 0.05 as I am curious to see the impact on the CPU, however I must admit some surprise on how bad (slow) the performance of the emulation is!

…cted

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
…rver

Add a new runtime_warning() method that will add a 'runtime_warning'
type message to the printer.configfile.warnings object.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
@KevinOConnor KevinOConnor force-pushed the work-mcufreq-20240305 branch from a84aa64 to 0291a15 Compare March 14, 2024 01:44
@KevinOConnor KevinOConnor merged commit 0291a15 into master Mar 14, 2024
2 checks passed
@KevinOConnor KevinOConnor deleted the work-mcufreq-20240305 branch March 14, 2024 01:45
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.

4 participants