sb0 changed the topic of #m-labs to: ARTIQ, Migen, MiSoC, Mixxeo & other M-Labs projects :: fka #milkymist :: Logs http://irclog.whitequark.org/m-labs
<sb0>
rjo, so what's the plan to fix the FUD issue, greenwire the existing PCBs?
<sb0>
and won't a global FUD cause problems when using the profile pins later? maybe the DDS select should be one-hot encoded?
<rjo>
iirc the profile pins don't need fud (they have that implicitly)
<sb0>
ah, yes
<rjo>
do you mean actual individual fuds per dds? i think that does not scale well enough.
<sb0>
yes, or individual analog switch enable, but same problem
<rjo>
but conceptually the profile selects should be handled like fud. either everything individual or everything decoded by select.
<sb0>
don't you need to hold them stable?
<sb0>
though you can do that with a latch instead of a switch
<rjo>
yes.
<rjo>
ah well. let's leave it at individual fuds. from the code it looks like you can adjust the pow based on the position in the batch and then iterate the fud through the affected dds.
<rjo>
iterate through the batch again and do the fuds.
<rjo>
that way the fuds are not coincident at the "logical time" of the batch but as closely spaced as possible.
<sb0>
cr1901_modern, "Curiously enough, not network byte order..." =
<sb0>
?
<cr1901_modern>
I expected the data over the socket to be sent in big endian order, but it's little endian
<sb0>
hm, what data?
<cr1901_modern>
int32's in particular
<cr1901_modern>
(and int16, which I added for WIndows)
mumptai has quit [Ping timeout: 252 seconds]
<cr1901_modern>
Did you design the little protocol that migen uses to send/rcv data to the VPI?
<sb0>
yes
<cr1901_modern>
I have a tendency to write comments to myself in code that I write. Since I usually work alone, it's not a problem. That's a habit I need to stop and is NOT a criticism of your design decisions
<sb0>
I don't remember a particular reason for using LE...
<cr1901_modern>
probably is what you're used to maybe? In any case, it did throw me off when adding to the inteface initially, but the comment should be sufficient if anyone else needs to hack on the C cod
<sb0>
the pack* functions should probably use bytes directly instead of going through a list
<cr1901_modern>
Sorry, can you please elaborate?
<sb0>
the _pack family of functions add elements to a list, which is then converted to bytes
mumptai has joined #m-labs
<sb0>
1) this is inefficient 2) this precludes the use of struct.pack() which would replace _pack_int32 and _pack_int16 with a one-liner
<cr1901_modern>
I see. The pack family code was there before I started hacking on it
<cr1901_modern>
I can change it to use struct.pack. I just wanted to make the minimal amount of changes to get a working implementation :P.
<sb0>
"unsigned short will be (better be) 16-bits on Windows, otherwise there's trouble!"?
<sb0>
afaict we can just use int
<cr1901_modern>
Okay, I think that comment's out of date
<cr1901_modern>
initially I tried big-endian stream shifting. That means buffer[1] would be right shifted, right?
<sb0>
cr1901_modern, what if the initial recv() returns 1 byte?
<cr1901_modern>
Oh... yea. That would be a problem.
<sb0>
"received_num will never exceed MAX_LEN" unless, e.g., if the initial recv() returns 1 byte and received_num is broken
<sb0>
i.e. la foire aux bugs
<cr1901_modern>
la foire aux... *reference zooms over my head*
<cr1901_modern>
Yes, as the code stands, I should amend that comment :P
<sb0>
why do you have to deal with the value of "l" differently in MESSAGE_GO than you do in other messages types? this is smelly
<cr1901_modern>
well the assert(l == 1) was there already. The assert(l == 3) is because the minimum valid message, disregarding the bugs above, is 3 on Windows
<cr1901_modern>
2 bytes for length, 1 byte for the message type
<sb0>
sure but why are the other messages types using l without caring about the extra size field?
<sb0>
this looks dirty, and i suspect bugs
<cr1901_modern>
i is incremented to 2 to account for this in the windows code
<cr1901_modern>
before the switch(buffer[i++])
<cr1901_modern>
I'm thinking of a better way to do it
<sb0>
ah, right
<cr1901_modern>
My rationale of course, was to limit the amount of code affected by platform specific stuff. The meaning of "i" and "l" are the same in both _WIN32 and UNIX
<sb0>
also you should not switch roles for "l"
<sb0>
in the windows code, l is the number of bytes received by one recv call, and received_num is the total number of bytes
<sb0>
and then l becomes received_num
<sb0>
assert((l-i) == 0) ?
<cr1901_modern>
I'll add a new var for it.
<cr1901_modern>
Were did I do that?
<sb0>
l = recv(..., received_num += l;, l = received_num;
<sb0>
no, but that's a way of removing the #ifdef and making clear what i and l do in a platform independent way
<cr1901_modern>
Oh! okay... thought you were pointing out something I wrote.
<cr1901_modern>
... why did I even do that in the first place?
<cr1901_modern>
In the Unix code, l is the total number of bytes received
<cr1901_modern>
this works b/c SOCK_SEQPACKET actually exists and will wait until the whole packet is received
<cr1901_modern>
I guess I took the "code reuse" a bit too far XD
<sb0>
please remove all trailing whitespaces, too
<cr1901_modern>
What about the Python packing functions. They were there before I started working on the code. Do you want me to replace them with compiled structs?
<sb0>
"signed int to char conversion will truncate bits due to possible right-shift sign extend."?
<sb0>
yes, there were there before, and yes replacing them would be nice
<cr1901_modern>
C doesn't specify whether right shift sign extends or not
<sb0>
well the sign bit should not be set
<sb0>
and "0xFF00 &" strips it anyway
<cr1901_modern>
and actually... I think the results of (0xFF00 & len) is unsigned anyway
<sb0>
the function replacement should be a separate patch
<sb0>
(struct.pack)
<sb0>
cr1901_modern, I've done some cleanups on your code and will send you a patch to apply after yours. can you integrate it and resubmit the whole thing?
<cr1901_modern>
Yea sure ting
<cr1901_modern>
thing* even
<cr1901_modern>
Okay, sounds good. Just need to tweak the C code and send off a new patch. I'm still not sure where the assert((l -i) == 0) goes however
<cr1901_modern>
Taking a short break- will be back in 30
<cr1901_modern>
Ahhh I see: case MESSAGE_GO: assert((l - i) == 0); Not sure if I would've thought about that, tbh
<cr1901_modern>
sb0: Could you run the following shell statement from within the examples/sim directory of migen:
<cr1901_modern>
for i in *.py; do python3 $i; done
<cr1901_modern>
I'm getting an "assertion failure, followed by WinError An existing connection was forcibly closed by the remote host"- it seems that MSYS is closing the socket too soon. So technically, I guess the code is doing what it's supposed to do.
<cr1901_modern>
(I was using this just to test the simulations before I commit lol)
<cr1901_modern>
All of them pass, aside from that curious error. Can't test fir b/c I don't have scipy
<GitHub148>
[artiq] sbourdeauducq pushed 2 new commits to master: http://git.io/vUm6M
<GitHub148>
artiq/master fdc406f Sebastien Bourdeauducq: transforms/inline: support user-defined context managers
<GitHub148>
artiq/master 1ceb06f Sebastien Bourdeauducq: dds: use context manager for batches
<sb0>
i still think that not syncing the DDSes is a bad idea, because even if you don't get metastability from the setup time violation (according to Chris from Oxford, it seems the chips are pretty resistant to that), jitter may still cause the FUD to happen intermittently in one cycle of the next and break phase
<sb0>
*or the next
<cr1901_modern>
I'm sure you've heard this before, but I can't resist read "FUD" as "Fear, Uncertainty, and Doubt"
<cr1901_modern>
reading* even
<sb0>
given the design of the current electronics, that's a fair interpretation of it. hopefully, it'll mean "Frequency UPdate" in the next HW version.
<cr1901_modern>
I think my problem with modern technologies is that I have trouble tackling their complexity into manageable chunks
<GitHub10>
[artiq] sbourdeauducq pushed 1 new commit to master: http://git.io/vUYI9
<GitHub10>
artiq/master ce4b573 Sebastien Bourdeauducq: runtime: reset all DDSes upon startup
<GitHub196>
[artiq] sbourdeauducq pushed 1 new commit to master: http://git.io/vUYql
<GitHub196>
artiq/master 0ca42db Sebastien Bourdeauducq: runtime/dds: send one FUD per command in a batch, compensate POW
<GitHub18>
[pyparser] whitequark pushed 3 new commits to master: http://git.io/vUYl2
<GitHub18>
pyparser/master 7e7358a whitequark: Improve docs on pyparser.parse.
<GitHub18>
pyparser/master 4f53372 whitequark: Don't tie ast to Python's builtin ast.