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

Fails on FY2320 v3.4 #4

Open
psychogenic opened this issue Oct 11, 2021 · 23 comments
Open

Fails on FY2320 v3.4 #4

psychogenic opened this issue Oct 11, 2021 · 23 comments

Comments

@psychogenic
Copy link
Collaborator

psychogenic commented Oct 11, 2021

Greets Matt,
This library is one of the reasons I got myself an FY2320, but it simply didn't work at all.
I did a little digging and discovered two reasons things were failing:

  1. it only works at 9600bps; and
  2. the blasted device doesn't terminate every response with a newline

So I forked the code and created a branch that I can now use with the device. You can see it here:
https://github.com/psychogenic/fygen/tree/fy2320v3p4
I tried to do it in a way that would not affect any of the existing functionality, but its a quick and dirty job.
The reason I'm posting here is I'm wondering:

  • how am I the only one this happened to? Is my firmware (3.4) funky?
  • all the docs I find online state the protocol to be at 9600bps, which 2300s work @ 115200?
  • whether I should put the effort to clean up the code and deal with arbitrary waveform uploads (which I'm not certain are working yet), for a pull request.

In any case, it does what I need now. But if you think the pull might be included, I'll polish and test deeper.

Thanks for your library.
Cheers,
Pat Deegan

@mattwach
Copy link
Owner

Hey Pat,

I don't know anything about the FY2320 but I'm happy that you were able to get it working!

I'd be happy to take pull requests that make the FY2320 work in the base library. It seems like a combination of documentation updates (i.e. the baud rate) and some logic switches based on the device_name should do the trick. I think that if we can break up the changes into several small iterations, it will be easier for me to test each change against my FY2300.

Thanks for reaching out!

@psychogenic
Copy link
Collaborator Author

Hi Matt,

In short, all I did was added optional params to the constructor

fy = fygen.FYGen('/dev/ttyUSB0', debug_level=1, 
            baudrate=9600, 
            unterminated_responses=True)

changed the _recv() to deal with unterminated_responses manually, made the difference between a blind transmission of data vs a send+receive explicit throughout and had to put a bit of delay in a couple of spots when using read_before_write=False (it's horrendously slow, for me, to be checking the values prior to writes, compared to just overwriting with same).

Doing a make test passes all 112 for python3, so I didn't hurt anything with default settings, it seems.

But if you can tell me how you'd like to see it broken down, I can probably do that this week.
Thanks

@mattwach
Copy link
Owner

That's good that the tests are passing.

I feel like the baud rate and read_before_write issues could be addressed with updated documentation. I can add some notes to the introduction and troubleshooting sections and send them to you for review.

The change to unterminated_responses could be it's own change. I think that using device_name to automatically set the flag when it's not manually specified could be good, although it does involve assuming that this is a FY2320 issue and not a firmware level one. I can refine things as people report in.

Maybe we can do those things as a first step.

@skua47
Copy link

skua47 commented Oct 14, 2021

Hi Matt,
I jump in to say that I had the same problem with my FY2320 V2.0 system. I got the communication to work by changing the baud rate in fygen.py to 9600. However, the behavior is still not very satisfactory. For example, running the simple.py script does work to set the function. The issue is that it is extremely slow to do so. It takes more than 2 minutes to finish the waveform changing process.
Here is the output of simple.py (I changed the frequency). Every line output takes about 5-10 s to appear
RMD ->
RMD -> 500
RMP -> 0
RMO -> 1000
RMA -> 300
RMF -> 0001000.000000
RMW -> 1
RMN -> 255
WMD50.0 ->
WMD50.0 ->
WMD50.0 ->
WMD50.0 ->
WMD50.0 ->
WMD50.0 ->
WMO0.00 ->
WMO0.00 ->
WMO0.00 ->
WMO0.00 ->
WMO0.00 ->
WMO0.00 ->
WMA2.00 ->
WMA2.00 ->
WMA2.00 ->
WMA2.00 ->
WMA2.00 ->
WMA2.00 ->
WMF00001230000000 ->
WMF00001230000000 ->
WMF00001230000000 ->
WMF00001230000000 ->
WMF00001230000000 ->
WMF00001230000000 ->
WMW28 ->
WMW28 ->
WMW28 ->
WMW28 ->
WMW28 ->
WMW28 ->
RMD -> 500
RMO -> 1000
RMA -> 200
RMF -> 0001230.000000
RMW -> 28
WMD50.0 ->
WMD50.0 ->
WMD50.0 ->
WMD50.0 ->
WMD50.0 ->
WMD50.0 ->
WMO0.00 ->
WMO0.00 ->
WMO0.00 ->
WMO0.00 ->
WMO0.00 ->
WMO0.00 ->
WMA2.00 ->
WMA2.00 ->
WMA2.00 ->
WMA2.00 ->
WMA2.00 ->
WMA2.00 ->
RMD -> 500
RMO -> 1000
RMA -> 200
WMD50.0 ->
WMD50.0 ->

@psychogenic
Copy link
Collaborator Author

psychogenic commented Oct 14, 2021

Hi @skua47
Ok, so I'm not alone in the world! ;-)
yeah, it's timing out on every read, and not getting anything back on writes.

All that is fixed in my fork. You can get details, update source, and usage for my modifications, here:
https://inductive-kickback.com/2021/10/controlling-a-feeltech-function-generator-with-a-computer-issues-and-fixes/
or just watch the adventure and results in video format:
https://www.youtube.com/watch?v=MxBm_r2cTUU

@mattwach
Copy link
Owner

Hi Pat, the link you posted above is giving me a 404 error.

As for speeding things up, there are a couple of settings to experiment with. Pat's code might already handle this.

timeout: The default is 5. Try a lower value, like 1 or even 0.25
read_before_write: Try setting it to "False"

i.e.

fy = fygen.FYGen(read_before_write=False, timeout=1)

This is in addition to the baud rate change, which I'll add as an additional constructor parameter.

Of course, timing out every command is not a great solution. The logic would need something to determine a good response though. Maybe there could be a setting to assume the command is done without seeing the newline. Pat might know more about the specifics.

@psychogenic
Copy link
Collaborator Author

psychogenic commented Oct 14, 2021

404: ugh, stale DNS (in the process of migrating the webserver, so you're seeing the old site). Anyhow, the video @ youtube works.

Short version is that this is all handled in the new _recv(), specifically:
https://github.com/psychogenic/fygen/blob/fy2320v3p4/fygen.py#L1198

So, it times out, but only once it's no longer getting characters and it's much faster.

@psychogenic
Copy link
Collaborator Author

Ok, FYI brought the old server I thought would be ignored up to speed with new post,
https://inductive-kickback.com/2021/10/controlling-a-feeltech-function-generator-with-a-computer-issues-and-fixes/
should work, regardless of DNS, now.

@skua47
Copy link

skua47 commented Oct 14, 2021

Alright, Thanks Pat, I'll try your fork and get back to you. Looking forward to getting this working, the library looks really nice, thanks for doing this Matt. My interest is to use the awg functionality to set up electrochemical excitation waveforms for our research lab.
David

@mattwach
Copy link
Owner

I created #5 to add the baud rate option. PTAL if you have a chance.

@psychogenic
Copy link
Collaborator Author

Hey @skua47
The arbitrary waveform settings are the one thing I haven't tested. Gut feeling is that it'll need some sort of delay in the writes to give it time to deal with the data, but I don't know.
If you do hit any issues that you can't resolve, shoot me some sample source to replicate.

@mattwach
Copy link
Owner

Pat, web page loads for me now. Thanks.

@psychogenic
Copy link
Collaborator Author

@mattwach The only issue I foresee with my branch might be if the device responds to (blind) sends with something and that rx data interferes with the next read or whatever (my device says nothing for WMN0 and stuff like that, not sure what yours does).
Fix for that would be to clear out the inbuffer for every new read.

@mattwach
Copy link
Owner

@psychogenic I changed the _recv code a little for better reporting basically adding:

[int(c) for c in response.encode()]))

to the debug message.

Then I ran the following:

$ ipython3
Python 3.9.7 (default, Aug 31 2021, 13:28:12)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.27.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import fygen

In [2]: fy=fygen.FYGen(debug_level=1)
UMO -> FY2350H [70, 89, 50, 51, 53, 48, 72, 10]

In [3]: fy.send('UMO')
UMO -> FY2350H [70, 89, 50, 51, 53, 48, 72, 10]
Out[3]: 'FY2350H'

In [4]: fy.send('WMW01')
WMW01 -> [10]
Out[4]: ''

So my unit is sending a single LF character at the the end of it's reply. For "write" commands, the LF is the only thing sent back.

@psychogenic
Copy link
Collaborator Author

Ah, well great. That means the strip() should handle any dangling whitespaces. Lemme know if you hit any issues with my code with your device.

@mattwach
Copy link
Owner

@psychogenic

I want to make sure I understand the difference between our devices.

If you send your device the UMO command, does it reply with the model number and no terminating character?
If you send WMW01, does it respond with any data at all?

@psychogenic
Copy link
Collaborator Author

No, UMO is as expected ("FY2320\n"). The reads, however, return a value with no "\n". The writes, WMwhatever, return nothing at all.

@mattwach
Copy link
Owner

No feedback. That is going to be hard to make reliable :(

@psychogenic
Copy link
Collaborator Author

Yeah, it's not perfect. But we could implement a read_AFTER_write, and check that it's as expected. Haven't used it tons, but really it's working pretty nicely for set() and sweep stuff.

@psychogenic
Copy link
Collaborator Author

In fact, I may have been conservative with my timeout in the _recv(), and my main concern is the arbirtrary waveform writes. Gave it a shot once, the thing seemed to be waiting for more data, and then I moved on as I my current concern was switching between settings.

@mattwach
Copy link
Owner

The current code already does a read after write by default.

https://github.com/mattwach/fygen/blob/master/fygen.py#L383

It's hard to see in the code I guess, but that loop continues until the read says the setting is already set (unless read_before_write is false, in which case no reading is ever done)

example:

In [4]: fy.set(wave='sin')
RMW -> 1 [49, 10]
WMW00 ->  [10]
RMW -> 0 [48, 10]

In [5]: fy.set(wave='sin')
RMW -> 0 [48, 10]

The first try did a read, write and read-verify. Reexecuting did a read but no write since the setting was already as-needed.

Starting with read_before_write=False

In [6]: fy=fygen.FYGen(debug_level=1, read_before_write=False)
UMO -> FY2350H [70, 89, 50, 51, 53, 48, 72, 10]
...
In [8]: fy.set(wave='sin')

WMW00 -> [10]

no verification.

Sometimes my FY2350 silently ignores commands, so the read_before_write=True is needed for reliable operation of my device.

As for responding with LF my device sometimes does not respond either:

https://github.com/mattwach/fygen/blob/master/fygen.py#L315

I wonder if putting in a small delay after the send would give your device enough time to respond with something. Maybe it never will, but it's worth a try.

@psychogenic
Copy link
Collaborator Author

Ah, I started running with read_before_write False because of mah need4speed.

But I doubt it's about delayed newlines--the WMN0's were all timing out... if it's longer than 5 seconds, it isn't worth it anyway, would make it unusable.

So, I think the solution is to stop using read_before_write=False, and tighten up the _recv so it's less conservative (say sleeps less and/or gives fewer "attempts"/wait loops after last char received).

@mattwach
Copy link
Owner

Sounds good. Let me know when your branch is in a state you like and I'll try running it against my FY2350.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants