Skip to content
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

Adding user modifiable waveform output interface for real-time waveform output. #890

Closed
veripoolbot opened this issue Feb 27, 2015 · 12 comments
Closed

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Feb 27, 2015


Author Name: HyungKi Jeong
Original Redmine Issue: 890 from https://www.veripool.org
Original Date: 2015-02-27
Original Assignee: HyungKi Jeong


I use the GTKWave as waveform tool and I was needed to check verilator's wave-out in real-time for my project (http://sourceforge.net/projects/test-drive/).

Some years ago, I patching the verilated_vcd_c.h and verilated_vcd_c.cpp in every release for my source.

But I hope to add user modifiable file's output interface in verilator's 'VerilatedVcdC' class (in C++ include sources).

So I send the fixed some sources(verilated_vcd_c.h, verilated_vcd_c.cpp of verilator-3.870.tgz) on here. Please check this.

on youtube : https://www.youtube.com/watch?v=vYs_kJNy_pc&feature=player_detailpage (from 3:40~ it shows real-time waveform output capability with verilator.)

Thanks.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Feb 27, 2015


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-02-27T02:22:59Z


Good work.

I don't want to make all of verilated_vcd_h's members protected as that will mean your code and others will certainly break when I change any internals. It looks like if you simply make VerilatedVcdFile a class with those three members that doesn't inherit from VerilatedVcd you'll still get what you need. Can you try that and confirm?

Also a minor thing please lower case the first letter of the function names, and I'd suggest fd for file descriptor - "fdWrite" not "OnWrite", likewise "fdOpen", "fdClose".

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Feb 27, 2015


Original Redmine Comment
Author Name: HyungKi Jeong
Original Date: 2015-02-27T03:32:44Z


Wilson Snyder wrote:

Good work.

I don't want to make all of verilated_vcd_h's members protected as that will mean your code and others will certainly break when I change any internals. It looks like if you simply make VerilatedVcdFile a class with those three members that doesn't inherit from VerilatedVcd you'll still get what you need. Can you try that and confirm?

Also a minor thing please lower case the first letter of the function names, and I'd suggest fd for file descriptor - "fdWrite" not "OnWrite", likewise "fdOpen", "fdClose".

I've understand your intension.
It was just for test, you can revert to 'private:'.
And I fixed function names likewise vcdOpen, vcdClose.

Thanks.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Feb 27, 2015


Original Redmine Comment
Author Name: HyungKi Jeong
Original Date: 2015-02-27T03:58:20Z


HyungKi Jeong wrote:

Wilson Snyder wrote:

Good work.

I don't want to make all of verilated_vcd_h's members protected as that will mean your code and others will certainly break when I change any internals. It looks like if you simply make VerilatedVcdFile a class with those three members that doesn't inherit from VerilatedVcd you'll still get what you need. Can you try that and confirm?

Also a minor thing please lower case the first letter of the function names, and I'd suggest fd for file descriptor - "fdWrite" not "OnWrite", likewise "fdOpen", "fdClose".

I've understand your intension.
It was just for test, you can revert to 'private:'.
And I fixed function names likewise vcdOpen, vcdClose.

Thanks.

Sorry, it must be fixed in source verilated_vcd_c.h line 425.
425 if(pVcdOut)
=>
425 if(!pVcdOut)

Simply... fix as below.

     VerilatedVcdC(VerilatedVcd* pVcdOut = NULL) {
		m_pTrace	= pVcdOut ? pVcdOut : (new VerilatedVcdFile);
	}

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Feb 27, 2015


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-02-27T12:13:14Z


Please change

class VerilatedVcdFile : public VerilatedVcd

to

class VerilatedVcdFile

Then you can make a

class VerilatedVcdDiskFile : public VerilatedVcdFile

which implements the default actions (calls OS routines) when you pass NULL to VerilatedVcdC.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Feb 27, 2015


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-02-27T12:14:01Z


(That is, the relationship should be a VerilatedVcdC has-a file, not a file is-a VerilatedVcdC.)

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Feb 27, 2015


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-02-27T12:16:46Z


Actually I think you can skip the abstract class and just

change

class VerilatedVcdFile : public VerilatedVcd

to

 class VerilatedVcdFile

Then make the default methods to call OS routines.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 4, 2015


Original Redmine Comment
Author Name: HyungKi Jeong
Original Date: 2015-03-04T05:14:59Z


Then constructor of 'VerilatedVcdC' must change to this.

VerilatedVcdC(VerilatedVcdFile* pVcdOut = NULL) {
     m_pTrace    = pVcdOut ? pVcdOut : (new VerilatedVcdFile);
}
</code>

and user can make override class of 'VerilatedVcdFile' on your way.
Simple, looks good.
But one issue is left that 'VerilatedVcdC' needs the interface of 'VerilatedVcd'.
Anyway thanks. I'll wait next release. ;)

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 5, 2015


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-03-05T13:57:42Z


Pushed to git towards 3.871.

Thanks for the patches. I made a few renames, also note if you pass in your own class you need to destruct it yourself. (Honoring the usual rule that a library only frees what it allocs itself)

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 6, 2015


Original Redmine Comment
Author Name: HyungKi Jeong
Original Date: 2015-03-06T01:08:07Z


I've one more question, how can i override 'VerilatedVcdFile' on now.

In manual's example...

     #include "verilated_vcd_c.h"
     ...
     int main(int argc, char **argv, char **env) {
         ...
         Verilated::traceEverOn(true);
         VerilatedVcdC* tfp = new VerilatedVcdC;
         topp->trace (tfp, 99);
         tfp->open ("obj_dir/t_trace_ena_cc/simx.vcd");
         ...
         while (sc_time_stamp() < sim_time && !Verilated::gotFinish()) {
             main_time += #;
             tfp->dump (main_time);
         }
         tfp->close();
     }

This example shows the using of 'VerilatedVcdC' class not a 'VerilatedVcd'.
I can't find a way to override interface on this.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 6, 2015


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-03-06T01:22:45Z


I updated git again to fix that.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 6, 2015


Original Redmine Comment
Author Name: HyungKi Jeong
Original Date: 2015-03-06T01:33:31Z


Thanks~

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Apr 5, 2015


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-04-05T15:00:18Z


In 3.872.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.