Run test roms #47
Run test roms #47
Conversation
joajfreitas
force-pushed
the
run_test_roms
branch
from
10b4a2b to
1cc8864
Compare
joajfreitas
force-pushed
the
run_test_roms
branch
from
ed449ea to
c497e78
Compare
| @@ -591,7 +590,6 @@ struct Cli { | |||
| rom: Option<String>, | |||
| } | |||
|
|
|||
|
|
|||
| #[cfg(not(target_arch = "wasm32"))] | |||
There was a problem hiding this comment.
Reading this attribute is always a bit confusing to me.
Should we put a comment above saying "Desktop entry point"?
fpt/src/debugger.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub fn receive_command(&mut self, cmd: &DebugCmd) { |
There was a problem hiding this comment.
This signature matches the one in trait DebugInterface.
Why not put this in a impl DebugInterface for Debugger?
fpt/src/debugger.rs
Outdated
| self.breakpoints.push(Breakpoint { | ||
| pc: *pc, | ||
| active: true, | ||
| }); |
There was a problem hiding this comment.
What if there's already a Breakpoint with the same pc?
- Maybe we should test for the existence of a breakpoint on this
pcand do nothing in that case. - Maybe the field
breakpointsshould be a HashSet andBreakpointshouldimpl PartialEqto only compare thepcfield. - Maybe the field
breakpointsshould be aHashMap<u16, bool>that maps "program counters" to an active/disabled flag, or a simpler struct that for now only holds the active/disabled flag. The latter would allow us to extend the value later to hold, for example, a conditional expression. Or we could just make a separateConditionalBreakpointvariant.
fpt/src/debugger.rs
Outdated
| self.watchpoints.push(Watchpoint { | ||
| addr: *addr | ||
| }); |
There was a problem hiding this comment.
Same comment as above.
Do you think watchpoints could be extended in the future?
If not, watchpoints could be a HashSet<u16>.
fpt/src/lr35902.rs
Outdated
| impl DebugInterface for LR35902 { | ||
| fn receive_command(&mut self, cmd: &DebugCmd) { | ||
| self.debugger.receive_command(cmd); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
ooh, I see, this is delegating to DebugCmd, which happens to have a similar signature for now.
fpt/src/lr35902.rs
Outdated
| if self.pc() == 0xFE { | ||
| std::process::exit(1); | ||
| } |
There was a problem hiding this comment.
Why would the program counter jump to 0xFE?
There was a problem hiding this comment.
Some debugging I was trying that made its way here
There was a problem hiding this comment.
I commented a bit on the data structures on struct Debugger but this looks good to me anyways.
joajfreitas
force-pushed
the
run_test_roms
branch
5 times, most recently
from
5575408 to
e5f1905
Compare
joajfreitas
force-pushed
the
run_test_roms
branch
from
e5f1905 to
1b41b5b
Compare
No description provided.