Expose stuff #8

2022-12-31T11:27:15Z

TheZoq2 commented

2022-12-31 11:27:15 +00:00

(Migrated from github.com)

These are the changes I need for my experimental vcd viewer so far:

Deriving extra traits for the indices is needed for me to efficiently keep track of the last time a signal was changed
Last timestamp is needed to figure out how wide to make the default view
byte_len_of_string_tmstmp_vals_on_tmln.push(curr_tmstmp_len_u8); fixes a panic when indexing into string values
SignalValue and associated functions properly expose the current value of the signal

These are the changes I need for my experimental vcd viewer so far: - Deriving extra traits for the indices is needed for me to efficiently keep track of the last time a signal was changed - Last timestamp is needed to figure out how wide to make the default view - `byte_len_of_string_tmstmp_vals_on_tmln.push(curr_tmstmp_len_u8);` fixes a panic when indexing into string values - SignalValue and associated functions properly expose the current value of the signal

ThePerfectComputer (Migrated from github.com) reviewed 2022-12-31 11:27:15 +00:00

src/lib.rs

					
				@ -9,3 +9,3 @@

				pub use vcd::types::{Metadata, Timescale, Version};

				pub use vcd::signal::{Signal};

				pub use vcd::signal::{Signal, SignalValue};

ThePerfectComputer (Migrated from github.com) commented

2023-01-10 04:50:23 +00:00

OK - I see why you wanted to expose SignalValue: bde4c76b5e/src/translation/pytranslator.rs (L69)

That's not actually how I intended the API to be used... It's OK that you use it that way - I hadn't completed this project nor provided full example. What I probably had in mind was that Signal would always return a string, either of digits, or alphanumeric characters since the VCD viewer likely doesn't care about the actual numerical value.

Does waveview need to know about that actual values, or would accepting strings be OK?

It's also possible I hadn't entirely thought this through...

I can see a case where you might wish to display typing information in the waveform viewer itself.

OK - I see why you wanted to expose SignalValue: https://gitlab.com/TheZoq2/surfer/-/blob/bde4c76b5e7e0f57d9a551d6e4e567d30d027841/src/translation/pytranslator.rs#L69 That's not actually how I intended the API to be used... It's OK that you use it that way - I hadn't completed this project nor provided full example. What I probably had in mind was that Signal would always return a string, either of digits, or alphanumeric characters since the VCD viewer likely doesn't care about the actual numerical value. Does waveview need to know about that actual values, or would accepting strings be OK? It's also possible I hadn't entirely thought this through... I can see a case where you might wish to display typing information in the waveform viewer itself.

src/vcd/signal.rs

ThePerfectComputer (Migrated from github.com) commented

2023-01-10 04:19:05 +00:00

I'm not so sure that we can just ignore errors - my original intent in capturing them was to log them to perhaps some sort of logger buffer. That way, a given VCD viewer could pull up an error logger.

src/vcd/signal.rs

ThePerfectComputer (Migrated from github.com) commented

2023-01-10 04:36:35 +00:00

I think some of the VCD example I had for unit testing had aliases in them...
I'm not sure this can just be left unimplemented

I think some of the VCD example I had for unit testing had aliases in them... I'm not sure this can just be left unimplemented

BracketMaster commented

2023-01-03 18:59:47 +00:00

(Migrated from github.com)

I suppose not including the range in the signal name makes sense. I suppose that when I started working on Fastwave, I had assumed there'd be a tighter coupling between the frontend viewer and the backend.

Was including the range causing issues for you?

I suppose not including the range in the signal name makes sense. I suppose that when I started working on Fastwave, I had assumed there'd be a tighter coupling between the frontend viewer and the backend. Was including the range causing issues for you?

TheZoq2 commented

2023-01-03 19:04:28 +00:00

(Migrated from github.com)

It was, yes. The translation stuff i'm doing in python relies on the names being there in their pure form, and if I read the spec, it seems like the [] enclosed stuff shouldn't be included in the identifier

I'm open to actually saving them, but I forgot to do that before pushing btw

It was, yes. The translation stuff i'm doing in python relies on the names being there in their pure form, and if I read the spec, it seems like the [] enclosed stuff shouldn't be included in the identifier I'm open to actually saving them, but I forgot to do that before pushing btw

TheZoq2 commented

2023-01-09 17:46:25 +00:00

(Migrated from github.com)

I made another change to make the whole lib generic over the io::Read implementation, which allows it to be used with https://docs.rs/progress-streams/latest/progress_streams/ (and I guess any other weird streams we can think of)

Sign in to join this conversation.

No reviewers