CedarvilleCS / CedarLogic

CedarLogic: Free, Open Source Digital Logic Simulator
GNU General Public License v3.0
32 stars 19 forks source link

Logic: D Flip Flop - Low Active Set/Reset - set and clear inputs don´t work #34

Open casanova-uja opened 4 years ago

casanova-uja commented 4 years ago

Describe the bug "D Flip Flop - Low Active Set/Reset" (AE_DFF_LOW) set and clear inputs don´t work. They do nothing.

Screenshots image

Desktop (please complete the following information):

Product Version in Use

casanova-uja commented 4 years ago

I've detected where the bug is: In logic_gate.cpp, in the method Gate_REGISTER::hasClockEdge().

Only "syncLoad" is considered but we have to consider "syncClear" and "syncSet" too.

We can modify Gate_REGISTER::hasClockEdge() changing it by:

Gate_REGISTER::hasClockEdge(bool syncSignal) { return isRisingEdge("clock") && getInputState("clock_enable") != ZERO || !syncSignal; }

I don't know if I can update the repository in github, if I must create a brach or how to do it.

JoeHCQ1 commented 4 years ago

Hi @casanova-uja - Thanks for locating the bug, doesn't sound like too complicated a fix. If you'd like to submit a bug fix here's how you'd do that with GitHub:

See the GitHub official guide here

Only note I'd add is that after forking the project you can do your work directly on your local master branch - you don't have to create a new one. Once you push your changes up to GitHub (after testing them) you can come back to this repo's home page and there will be a button for you to create a pull request, do that, and I'll be able to review your changes.

Let me know if you run into any troubles, I've had a decent amount of GitHub experience at this point. One other thing that might help with the learning curve is that a fork of a repo is basically the same as a branch but you don't need any permissions to the destination repo to create a fork but you do for a branch.

JoeHCQ1 commented 4 years ago

@casanova-uja - feel free to also update the build instructions in the readme if you want - I'm very willing to accept help!

radiantly commented 4 years ago

Hi, could I take a shot at this issue?

JoeHCQ1 commented 4 years ago

@radiantly Absolutely! Go for it!

casanova-uja commented 4 years ago

I have a lot of corrections, changes and additions made on CedarLogic 2.3.5 but I don't know how to upload them to github.

smiddy007 commented 4 years ago

https://github.com/CedarvilleCS/CedarLogic/issues/2 ^^ they did do something, they acted as synchronous inputs regardless of whether or not the boxes were checked to be asynch or synch. @casanova-uja Any updates?

casanova-uja commented 4 years ago

This bug and many others has been resolved un casanova-uja fork (version 2.4.0) candidate.

JoeHCQ1 commented 4 years ago

@casanova-uja sorry for the late reply here:

If you go to your fork: https://github.com/casanova-uja/CedarLogic

you'll see the pull request button:

image

Click that and you'll open a pull request to this repo so we can review the changes. Please also leave a comment in the PR explaining your changes. I'd like to get to them soon but I'm busy renovating a house. @kshomper may be better able to review changes.

Thanks for your help!

casanova-uja commented 4 years ago

Hello:

I don´t know what is "Pull request". It is the first time I upload code to GitHub.

I am continually making modifications in the code to improve the program. From version 2.4.0, I have made more changes to version 2.4.1.

I would like you and the community to see the new capabilities and if you could give me your opinion and set a new version of CedarLogic in the future. Perhaps 2.5.0 will be a good number!

Also, I would like to change the program name to "UJACedarLogic" as I have collaborated to develop it. I don´t know if it is possible.

Thanks in advance and sorry for my bad english.

The link to installer of version 2.4.1 is: https://drive.google.com/file/d/17Ev5JYi2WO3vbTj06zbI0jOpWd0Svbqn/view?usp=sharing

For your information, the added capabilities are (till 2.4.1 version):

Flip Flop T (TFF) SR with habilitation (CONTROL) Repaired set and clear in FFD and registers Repaired Enabled output in register Repaired DECODER output size Label LINK with adjustable text size If you press Esc in save, do not erase the actual circuit Set Last Directory ini value to "My Documents" Bitmaps and Font in Resources, not in Files Eliminate res directory Change setttings file Change default "Display Wire Connections Points" to false In RAM popup adjusted data to number of bits, added Clear button Asyncrhonous RAM Deprecated components not necesary, now there is a new library for them Junctions in component connections points Wire to wire connection Collisions in menu and setting option default to false wireconnradius from 0.025 to 0.3, defautl 0.1 wireconnradius in toolbox slider and settings option Gridlines in toolbox View connection points in toolbox In Lock State not double click needed to change inputs Eliminate ENABLE, ENABLE_0 ... ENABLE_7 now use OUTPUT_ENABLE Add in decoder: ENABLE_A (for chip 74138) Changed to uppercase: write_enable, write_clock, clock_enable, clock, clear, set, load, count_enable, count_up, shift_enable, shift_left, carry_in, carry_out, signal, overflow, nQ, in_A_equal_B, in_A_greater_B, in_A_less_B, A_equal_B, A_greater_B, A_less_B, T_in, T_in2, T_ctrl Export to image in color, monochrome and greyscale Copy to clipborad in color, monochrome and greyscale Edit Menu options to copy bitmap to clipboard in monochrome, greyscale or color Autojunctions in RAM and REGISTER between IN and OUT bidirectionals (BIDIRECTIONAL_DATA) Eliminate 74138 chip it is 3x8 decoder Add NOT-BUF Gate_PASS with inverted output (OUTINV) for Registers and NOT-BUF Library (cl_gatedefs.xml) in resources and append external user library (UserLib.xml) Now permit LF (UNIX) and CRLF (WINDOWS) line terminators in libraries Settings in windows register Clear settings if new version Oscope admit TO, FROM and LINK Changed components distribution in libraries Added BCD only to REGISTER gate (for displays) Added HIDE_DISPLAY to REGISTER gate Modified installer script Register functions managed by opcode: 000: NOP, 001: LOAD, 010: INC, 011: DEC, 100: SLL, 101: SRL, 110: ROL, 111:ROR Version number do not include DATE and TIME Eliminated BUSEND gui type Changed BUS_END logic type to BUSEND Added a second connection point to BUSEND New for wide lines in gate shape New for wide lines in the outline of gate shapes, can be deactivated in settings menu and toolbar New for more wide lines for BUSEND Mirror for MUX, BUSEND and BUFFER, Use Control + mouse right button If newer version can continue if no incompatibility. If save update version number Eliminated "Contents..." and "Download latest version..." in "Help" menu

New Components

LUTs small input and output New junctions and wires New flip-flops LINK DEMUX

El lun., 2 nov. 2020 a las 18:55, Joseph Richardson (< notifications@github.com>) escribió:

@casanova-uja https://github.com/casanova-uja sorry for the late reply here:

If you go to your fork: https://github.com/casanova-uja/CedarLogic

you'll see the pull request button:

[image: image] https://user-images.githubusercontent.com/49208786/97901563-5baee880-1d0a-11eb-8234-dd5c377bbad2.png

Click that and you'll open a pull request to this repo so we can review the changes. Please also leave a comment in the PR explaining your changes. I'd like to get to them soon but I'm busy renovating a house. @kshomper https://github.com/kshomper may be better able to review changes.

Thanks for your help!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CedarvilleCS/CedarLogic/issues/34#issuecomment-720629408, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIYBGLTPXT74FXWMM5GR5GLSN3W7HANCNFSM4MSVI6GA .

--

No abro ni contesto correos marcados con mailtrack o similares.

[image: Universidad de Jaén] http://www.uja.es/ Pedro J. Casanova Peláez Profesor Titular de Universidad casanova@ujaen.es

Universidad de Jaén Departamento de Ingeniería Electrónica y Automática Edificio A3, despacho 424 Campus Las Lagunillas, s/n - 23071 - Jaén | +34 953 212 805 |

[image: Universidad de Jaén] http://www.uja.es/

JoeHCQ1 commented 3 years ago

Hi @casanova-uja sorry again for my late replies here.

So a pull request is how you merge changes from your fork of the repo back into this repo. This GitHub document might help if there's a spanish version available: https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-requests.

As far as the name change I don't think we're planning to put contributor names in the program name.

I'll go ahead and create the PR (pull request) for you and link it in a minute.

JoeHCQ1 commented 3 years ago

@casanova-uja here is the PR from your fork back into the main project.

JoeHCQ1 commented 2 years ago

Just tested this, the latest code in master has this bug fixed.

JoeHCQ1 commented 2 years ago

I'm really confused,

For one, the lastest code in master still has this bug.

Second, the recommended change:

Gate_REGISTER::hasClockEdge(bool syncSignal)
{
return isRisingEdge("clock") && getInputState("clock_enable") != ZERO || !syncSignal;
}

Seems to have been the code since this was added into GitHub.

To make it more confusing, git blame shows most lines as having never been committed. Makes me suspect that we lost the git history when we moved over from sourceforge. I could probably fix that by cloning from source forge and then replaying the current master on top of that, and force pushing to master (or main to keep up with the times).

kshomper commented 2 years ago

Joe ... I don't know enough about the various git processes so that your remedy (and associated risk of that remedy) is unfamiliar to me. Would you please reach out to me via email so we could set up a short phone conversation to discuss? Keith

JoeHCQ1 commented 2 years ago

@kshomper it would be a very risky maneuver however, GitHub shows the history for those files therefore I suspect the problem is actually with my git bash - the risky git history edits will not be necessary.

kshomper commented 2 years ago

@JoeHCQ1 sounds good. Glad the edits are unnecessary. Thanks for your work.