Xilinx / PYNQ

Python Productivity for ZYNQ
http://www.pynq.io/
BSD 3-Clause "New" or "Revised" License
1.99k stars 818 forks source link

PMOD_ALS does not work #39

Closed yunqu closed 8 years ago

yunqu commented 8 years ago

We need to solve this issue.

npurusho commented 8 years ago

Hi Rock,

Looking into this. I will Update soon.

Naveen

yunqu commented 8 years ago

After adding pmod_init() into the microblaze program, it is still not working.

cathalmccabe commented 8 years ago

In what way is it not working? It seems OK for me. Has it already been fixed?

npurusho commented 8 years ago

Hi Cathal,

I tested it out in SDK. It does not work when we hot plug. This is similar issue to the TMP2, for which as extra IIC read was added to get rid of the stale data. The same fix works for the ALS as well, performing 2 SPI reads.

I havent tested it out in python with the new bitstream. Naveen

yunqu commented 8 years ago

We still have this issue. Both Naveen and me could not get this working...

cathalmccabe commented 8 years ago

I've tested on board with current code on GitHub, and latest bitstream, both rebuilt locally, and it seems fine. Leave open if you wish, please check it yourselves, and if it is still not working, please provide the steps you take to demonstrate how it is not working.

npurusho commented 8 years ago

Hi Cathal,

The fail case is seen when we change iop(JB --> JC etc.) after bit file is loaded. The wrong value appears only the first time after we change. Image 2 is the failing case: both images should have output the same value(32)

Image 1: [cid:image001.png@01D19EFF.CBC305E0]

Image 2: please disregard comment in Cell 2 as ALS is on PMOD JC [cid:image002.png@01D19EFF.CBC305E0]

The way to reproduce this is: In the notebook: 1 . Execute cell one with ALS on JB

  1. Execute cell two with my_als = PMOD_ALS(1) - should expect image 1
  2. Change ALS to JC
  3. Execute cell two with my_als = PMOD_ALS(2) - should expect image 2

Also try it few times if you cannot reproduce it. Since the problem is intermittent.

This test is not a problem for TMP2. Since it has the redundant read. However the same fix does not seem to work for ALS.

Thanks & Regards, Naveen From: cathalmccabe [mailto:notifications@github.com] Sent: Monday, April 25, 2016 1:50 PM To: Xilinx/Pynq Cc: Naveen Purushotham; Comment Subject: Re: [Xilinx/Pynq] PMOD_ALS does not work (#39)

I've tested on board with current code on GitHub, and latest bitstream, both rebuilt locally, and it seems fine. Leave open if you wish, please check it yourselves, and if it is still not working, please provide the steps you take to demonstrate how it is not working.

— You are receiving this because you commented. Reply to this email directly or view it on GitHubhttps://github.com/Xilinx/Pynq/issues/39#issuecomment-214517080

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

yunqu commented 8 years ago

Naveen,

It would be better to comment directly on issue tracker, instead of replying the email. We are not able to see the rest part of the message.

npurusho commented 8 years ago

I had images in the message, tracker did not accept them. So I replied via email.

yunqu commented 8 years ago

capture

This is the issue. I did not even do hot swapping. I booted my board with ALS on PMOD 1. But the first read seems not right.

I used pip install update right before I run this.

yunqu commented 8 years ago

capture

Another example when running pytest.

schelleg commented 8 years ago

I made a byte ordering swap with masking and see good values with my iphone light and also with me holding finger over sensor.

Let me know if checked-in change fix works - I'm curious if this breaks things for Cathal but fixes Rock/Naveen.

https://github.com/Xilinx/Pynq/blob/master/zybo/sdk/pmod_als/src/pmod_als.c#L77

cathalmccabe commented 8 years ago

Hi Guys, Sorry, I didn't get to check this all week. I have a quick look at it and compared with the old version.

Graham, I checked with a scope (analog discovery). Not sure why you did the byte swap. I think it was correct the first time. This is odd though, as it worked for you Graham, but I got '0's.

We receive the data like this: 0000XXXXXXXX0000

raw_data[0] shoudl be 0000XXXX raw_data[1] should be XXXX0000

So looks like we need this:

( ((raw_data[1] & 0xf0) >> 4) + ((raw_data[0] & 0x0f) << 4) );

(Unless the bytes are swapped again elsewhere)

I also checked and compared the old version with the new as I thought the old one was working. The problem seems to be with the new SPI code. Parimal's new spi transfer was setting up the SPI in a slightly different way.

We write 0x18e to the CR register, before writing, and then write 0x8e (The 0x100 acts like and enable, disable)

In the old version, I was writing 0x186. The difference is the clock polarity (high/low - I'm not sure which is which without checking the data sheet).

There was also a spi delay loop. Originally I had a loop of i=0;i<9;i++. In Parimal's, this was shorted to 7.I'm not sure if this really makes a difference.

Making these changes fixed the reads for me (other than the wrong value for the first read when plugged in).

I fixed the wrong first value by doing an initial read at the start of the pmod_als code - like tmp2 I think.

I haven't checked anything in yet as it will affect other code (anything that uses SPI). I also didn't spend much time on this - I was rushing to look at it this afternoon. I would prefer to check it in a little more detail and check the data sheets to fully understand what is happening.

schelleg commented 8 years ago

Hi Cathal,

Can you send out a pmod_als.bin that we (Naveen/Rock/I) can test against?

For verifying - I want to verify we all consistently read ~0 when dark ~40 when ambient light and ~255 when flashlight shined on.

cathalmccabe commented 8 years ago

bin e-mailed. Let me know if you receive it.

yunqu commented 8 years ago

I tested Cathal's version sent on Friday. It appears to me that it works for Jupyter notebooks, or python commands, but not in pytest.

I am now able to see correct results in pytest by doing:

als = PMOD_ALS(als_id)

: Wait for the PMOD ALS to finish initialization

sleep(0.01) n = als.read()

So I guess the SPI device is too slow and this might be the same case for other SPI devices (e.g. TMP2, where I guess you don't really need 2 reads, but a longer delay).

I will upload the new files and close this issue soon.