Open Rodgers-PAC-Lab opened 2 years ago
I'll handle merging the bugfix from main and making it match here, sorry for the confusion. lot of things in the air at the moment with pending changes lol
Pulled to your branch, if that looks like it works, then feel free to accept it and then we can merge this one :) https://github.com/Rodgers-PAC-Lab/autopilot/pull/1
Awesome, thanks for doing all this to implement the filtering methods! On my end, it seems to work. I added a commit that adds documentation that would be helpful to me. Basically, the scipy.signal argument Wn is super confusing to me, sometimes they want it as a fraction of the Nyquist frequency (a major gotcha because it's half the sampling frequency) and the actual scipy.signal doc is hard for me to follow. For digital filters, Wn are in the same units as fs. By default, fs is 2 half-cycles/sample, so these are normalized from 0 to 1, where 1 is the Nyquist frequency. (Wn is thus in half-cycles / sample.)
The behavior changes depending on whether fs
is provided. Anyway, I suggest making the doc more explicit on our end to help with that.
Also, as I'm watching the networking DEBUG messages go by, I see that "highpass" is always null. I think this might be because it's an Filter_IIR object instead of a float? Not sure.
DEBUG:networking.station_02.Pilot_Station.rpi09:FORWARDING (dealer): [b'_rpi09', b'rpi09', b'T', b'{"id": "_rpi09_34", "to": "T", "sender": "_rpi09", "key": "DATA", "value": {"target": "L", "trial_num": 8, "correction": false, "duration": 100.0, "amplitude": 0.01, "highpass": null, "channel": 0, "type": "Noise", "pilot": "rpi09", "subject": "NoiseVsHpNoise"}, "timestamp": "2022-03-29T10:42:15.349901", "flags": {"NOREPEAT": true}, "changed": false, "ttl": 2}']
But, I can clearly hear that the filtering is being applied, so it seems to be working.
My previous PR https://github.com/wehr-lab/autopilot/pull/166 was against main instead of dev. This is the same, except against dev. Also, wehr-lab/dev seems to be missing the bugfix https://github.com/wehr-lab/autopilot/commit/90956187d4222f16f67ab8b39b8359da954d5dcc that is in wehr-lab/main and also in this PR. I think if that bugfix gets merged into wehr-lab/dev first, it'll make more sense. Sorry for the confusion, let me know if it'd be helpful for me to do something differnetly.
Notes from first PR: A new sound of type Tritone An argument "highpass" to the sound Noise which permits it to be optionally highpass-filtered If "highpass" is left blank, then Noise will function just as before.
See also issue https://github.com/wehr-lab/autopilot/issues/165