search

sharpie7 / circuitjs1

Electronic Circuit Simulator in the Browser
GNU General Public License v2.0
2.29k stars 633 forks source link

Add lineOver to ring counter's reset pin #606

Closed nulldg closed 3 years ago

nulldg commented 3 years ago

Circuitjs's ring counter has an active-low reset pin and is cleared with a low reset signal (this is distinct from the active-high reset pins of devices such as the CD4026BE). Unless I'm mistaken, I believe this should be marked with an overline. I spent a solid 10 minutes trying to figure out why the ring counter was not working - the lack of marking suggested the reset pin is active-high, not active-low. Related but not part of this commit: It would be nice to have an edit option to change whether or not the reset pin is active-high or active-low.

pfalstad commented 3 years ago

Hi, the reset pin has a bubble on it, which was supposed to indicate that it's active-low. A line over it would be better, but we should remove the bubble.

nulldg commented 3 years ago

Oh damn, I didn't notice the bubble. In practice, I've never seen datasheets or diagrams depict bubbled pins in place of overlines. I've made the relevant changes. Thanks for the quick response, paul!

nulldg commented 3 years ago

@pfalstad I just noticed this repo is several versions behind your fork. Should I make pull requests on your fork instead?

pfalstad commented 3 years ago

@nullg making pull requests to my fork is probably better, but it doesn't matter much. I can pull from wherever.

nulldg commented 3 years ago

Agreed. I want to start making pull requests on your fork rather than here, but I can't until my pull requests are accepted or rejected because I'm not able to keep two forks at once.

pfalstad commented 3 years ago

You need to wait until they're merged? That might take a while. Not sure when Iain will get around to it. I don't want to do it for him, since then the source wouldn't match what's on his web site. So you could continue to do pull requests here if it's easier.

nulldg commented 3 years ago

GitHub doesn't seem to allow duplicates, and changing my existing fork will likely break all the pull requests I've made so far. That said, if merging will take a while, I might as well close them and move the pull requests over.

pfalstad commented 3 years ago

I have already merged them all into in my local sandbox so you can just close them all.