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
Implement 16-bit timers #30
Conversation
@gfeun can you please check you code with this new branch and report? |
Hey @urish, So the timer1 works now ... but like a 8 bit one 😃 When i set So I think it comes from the fact that internally all timer handling functions still assumes 8 bit: avr8js/src/peripherals/timer.ts Line 53 in cf07a26
It should be 16bits on 0x89 0x88, see p144 of the doc Same for TCNT register: avr8js/src/peripherals/timer.ts Line 56 in cf07a26
avr8js/src/peripherals/timer.ts Line 93 in cf07a26
It should be 16 bits across 0x85 and 0x84, see p143 of doc I don't understand how avr8js/src/peripherals/timer.ts Line 196 in cf07a26
avr8js/src/peripherals/timer.ts Line 234 in cf07a26
avr8js/src/peripherals/timer.ts Line 319 in cf07a26
|
Hi Glenn, thanks for the detailed analysis! Actually, It probably has to be with the fact that we add CPU watches for a single address: avr8js/src/peripherals/timer.ts Line 201 in cf07a26
I will update the code there to support 16 bit timers as well |
Thanks for the explanation ! Interesting to hear about assembly script, I'm learning about Web Assembly for work at the moment. I'm going to test modifying CPU watches on my side too |
@gfeun I have updated the branch with some fixes, so hopefully now OCR1A should function as expected. Can you please have a look on the updated code? |
I think it works with the following changes: diff --git a/src/peripherals/timer.ts b/src/peripherals/timer.ts
index ea66671..b3bc770 100644
--- a/src/peripherals/timer.ts
+++ b/src/peripherals/timer.ts
@@ -237,7 +237,7 @@ export class AVRTimer {
set TCNT(value: u16) {
this.cpu.data[this.config.TCNT] = value & 0xff;
if (this.config.bits === 16) {
- this.cpu.data[this.config.TCNT + 1] = (value >> 16) & 0xff;
+ this.cpu.data[this.config.TCNT + 1] = (value >> 8) & 0xff;
}
}
@@ -281,7 +281,7 @@ export class AVRTimer {
private registerHook(address: number, hook: (value: u16) => void) {
if (this.config.bits === 16) {
this.cpu.writeHooks[address] = (value: u8) => {
- hook(this.cpu.data[address + 1] | value);
+ hook((this.cpu.data[address + 1] << 8) | value);
};
this.cpu.writeHooks[address + 1] = (value: u8) => {
hook((value << 8) | this.cpu.data[address]); |
src/peripherals/timer.ts
Outdated
set TCNT(value: u16) { | ||
this.cpu.data[this.config.TCNT] = value & 0xff; | ||
if (this.config.bits === 16) { | ||
this.cpu.data[this.config.TCNT + 1] = (value >> 16) & 0xff; |
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.
this.cpu.data[this.config.TCNT + 1] = (value >> 16) & 0xff; | |
this.cpu.data[this.config.TCNT + 1] = (value >> 8) & 0xff; |
src/peripherals/timer.ts
Outdated
private registerHook(address: number, hook: (value: u16) => void) { | ||
if (this.config.bits === 16) { | ||
this.cpu.writeHooks[address] = (value: u8) => { | ||
hook(this.cpu.data[address + 1] | value); |
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.
hook(this.cpu.data[address + 1] | value); | |
hook((this.cpu.data[address + 1] << 8) | value); |
Tested successfully with the following code runnable in the demo, and including my 2 changes, good job ! void setup()
{
// Builtin led as output
DDRB |= 0x20;
// initialize Timer1
cli(); // disable global interrupts
TCCR1A = 0; // set entire TCCR1A register to 0
TCCR1B = 0; // same for TCCR1B
// set compare match register to desired timer count:
OCR1A = 15624;
// turn on CTC mode:
TCCR1B |= (1 << WGM12);
// Set CS10 and CS12 bits for 1024 prescaler:
TCCR1B |= (1 << CS10);
TCCR1B |= (1 << CS12);
// enable timer compare interrupt:
TIMSK1 |= (1 << OCIE1A);
// enable global interrupts:
sei();
}
void loop()
{
}
ISR(TIMER1_COMPA_vect)
{
PORTB ^= 0x20; // Toggle builtin led
} |
Yay! Thanks :) Can you try to come up with a failing test case(s) for the changes that you suggested? |
(I mean tests that fail with the current code, and pass after applying your changes) |
Update: I was wrong, this test passes for both with and without my modification. it('should set OCF0A flag when timer equals OCRA (16 bit mode)', () => {
const timer = new AVRTimer(cpu, timer1Config);
cpu.writeData(0x84, 0xff); // TCNT1 <- 0x00ff
cpu.writeData(0x85, 0x00); // ...
cpu.writeData(0x88, 0x00); // OCR1A <- 0x0100
cpu.writeData(0x89, 0x01); // ...
cpu.writeData(0x80, 0x0); // WGM1 <- 0 (Normal)
cpu.writeData(0x81, 0x1); // TCCR1B.CS <- 1
cpu.cycles = 1;
timer.tick();
expect(cpu.data[0x36]).toEqual(2); // TIFR0 should have OCF0A bit on
expect(cpu.pc).toEqual(0);
expect(cpu.cycles).toEqual(1);
}); |
e.g. Timer/Counter1 on ATmega328
also fix some issues found by @gfeun and the tests
Alright, I applied the fixes, added tests, fixed a few more issues I found - I think this is ready to go! |
Great, this also works on my side. Thanks for finishing this, i couldn't write a test failing for the invalid code. |
Lovely! I will merge and release soon. |
Released as part of avr8js 0.8.0 |
See #29 for details