-
Notifications
You must be signed in to change notification settings - Fork 36
Switched to embassy_time 0.4.0 and embassy_executor 0.7.0 #95
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
Conversation
src/embassy/time_driver_tim.rs
Outdated
return false; | ||
} | ||
|
||
// Write the CCR value regardless of whether we're going to enable it now or not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation should be fixed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thank you. Used an auto formatter.
I @joao404 Nice changes! I have tested on CH32V003 and a basic blinky and it freezes after a couple of seconds: Example from ch32-hal-template: #![no_std]
#![no_main]
#![feature(type_alias_impl_trait)]
use embassy_executor::Spawner;
use embassy_time::Timer;
use ch32_hal::gpio::{AnyPin, Level, Output, Pin};
use ch32_hal::println;
use panic_halt as _;
#[embassy_executor::task]
async fn blink(pin: AnyPin, interval_ms: u64) {
let mut led = Output::new(pin, Level::Low, Default::default());
loop {
led.set_high();
Timer::after_millis(interval_ms).await;
led.set_low();
Timer::after_millis(interval_ms).await;
}
}
#[embassy_executor::main(entry = "qingke_rt::entry")]
async fn main(spawner: Spawner) -> ! {
ch32_hal::debug::SDIPrint::enable();
let p = ch32_hal::init(ch32_hal::Config::default());
// Adjust the LED GPIO according to your board
spawner.spawn(blink(p.PC4.degrade(), 1000)).unwrap();
loop {
Timer::after_millis(1000).await;
println!("tick");
}
} Output:
Edit: I have tried on a CH32V307 and it seems to work (ran for 2 hours before I stopped). Edit 2: In the case of the V003 freeze: wlink regs
wlink status
`mepc : 0x00000992`
|
Hi, unfortunately I do not have a wch-link to try it out myself. I thought I could use my ST-Link and did not order one. I have a ch32v307vct6 dev board and use wchisp (which works great by the way, thank you for providing it). I tried it using a blocking and async UartTx connection and had no problem for 10 minutes(then I stopped testing). I will try to find a solution with your debug information |
Ok, I see. |
If I understand documentation correctly, highest bit set means that we have an interrupt and 0x26 is number of interrupt. Compared with https://github.com/ch32-rs/ch32-data/blob/main/data/interrupts/CH32V0.yaml it should be TIM2, which has thrown interrupt. This fits to our Cargo.toml where "time-driver-tim2" is activated for ch32 hal. |
Hi @joao404 |
I have tested it on CH23X035 too but currently the examples do not set any When using the |
Hello, I received the wlink but was not able to test it with sdi print yet. I checked systick file before but it looks not like an easy merge/change. I will take a look at it after my vacation. Best regards |
Hi, I made the first changes to adapt time_driver_systick, but it does not work yet. I also included changes of #88. Maybe someone can have a second look. SDI print does not work with my probe so I have no good way to debug it at the moment. |
Hello, I was able to make systick work but with a change to embassy_time. From my point of view we have a timing problem. Default frequency of hclk(and thereby cpu) is 8Mhz so systick runs with 1 Mhz. Default embassy tick frequency is 1 Mhz. Thereby we have to check for ready futures every systick. The problem occurs from my point of view at the end of on_interrupt. The next interrupt is only fired, if we hit new compare register value. If counter value is already higher, no interrupt is given. So IMHO we only have 7 cpu cycles to calculate the next cmp register value which is only 1 counting value away of systick timer. Only if I reduce embassy_tick to lower values does it work properly. I debugged it using println! which showed that otherwise we often have a cmp register value higher than current counting value. |
Hi @joao404 It seems that SysTick can be driven by HCLK or HCLK/8. Maybe we can use it with HCLK directly without the divider when the systick feature is used for embassy_time? |
Hi @romainreignier , |
I think your assessment of the problem is correct, we should not attempt to run at 1MHz tick rate when our clock is only 8Mhz. That's insane overhead and almost guarantee we don't be running efficiently or even on time. |
I would propose to switch to a default tick of 10kHz, change Cargo.toml of every example for that and mention it in Readme. 10kHz worked for me during debugging. Do you agree? P.s.: also previous versions used 1Mhz tick. That it works irritates me a little. I also tried to make example spi-lcd-st7735-cube build for ch32v003 work, but I am not able to make it fit into flash even with optimization level z. All other versions are ok. |
Hello, I am able to build spi-lcd-st7735-cube for ch32v003 in release mode now but as before, it is a tight fit. Only a few bytes left. This seems to be a base embassy problem as mentioned in embassy-rs/embassy#4290. |
I have started the workflow. |
I understand now, that the failing action is not building examples but repo without any features. I thereby removed my compiler warning because it was not present before. Tried it with the same commands as github action. |
The alternative to remove your warning would be to add time driver features in CI for targets without 64-bit systick. I have tried again on the V003 and indeed, I had to increase the arena-size to 256 for it to run, we should increase the default value (it also means the new embassy stack uses more stack than before because it used to work with 128 I think). But now, I have this runtime error:
|
Changing arena-size to 256 does not work for me. Blinky does not work when I add it. @romainreignier can you tell me, how I should configure my workspace so that I can see such a panic with Wch linkE? |
I use the default config:
I have not tested the V307 with the latest version of this PR but last time, it was working on V307 and only failed on V003. |
Ok. So I am using the same mechanism but getting no error. ch32v307 run for 3 hours today without any problem. Because I have no ch32v003 I cannot debug it's problem. How do we want to proceed? |
Actually took a closer look with @Dummyc0m, the |
We have a patch here: https://gist.github.com/Codetector1374/880a58c69127c540747b8acbf1183fbb feel free to update your PR with this and test it on the boards you have (As @Dummyc0m and I only have CH32V305/307). We have verified this patch works with a simple embassy blinky example (like the one you have above) on our board. Let me know how it goes, would like to get this fixed/merged soon as well! |
Please take a look at this comment. Can you also rebase your patches on top of the upstream main branch? The merge commit cannot be rebased. Also please see the build error. |
Build error is fixed but I am fighting with the rebase. I am sorry. This is the first rebase I'm doing. |
One way to do it is to checkout the upstream in a new branch and
cherry-pick your commits in order.
git rebase -i is useful too if you want to squash/reorder commits.
…On Fri, Jul 4, 2025 at 15:03 Marcel Maage ***@***.***> wrote:
*joao404* left a comment (ch32-rs/ch32-hal#95)
<#95 (comment)>
Build error is fixed but I am fighting with the rebase
—
Reply to this email directly, view it on GitHub
<#95 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB4URTKAJKLYLXA45ZF5E2D3G32ZLAVCNFSM6AAAAAB42TBCISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAMZXGI4TSOBRHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Removed compiler warning in mod.rs of embassy for timer config. If no timer is defined for a none qingke_v4 processor, an error was originally given. This was not present in original code so I removed it again as it is blocking building embassy without any features
Hello, it looks, like I have done it |
Hello,
I have adapted time_driver_tim.rs in the same manner as time_driver.rs is for stm32 in embassy repo. embassy_time with version 0.4.0 should now be possible as requested by #82