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

CCD100: test mk3 protocol file with mk2 device #7005

Closed
FreddieAkeroyd opened this issue Feb 6, 2022 · 13 comments
Closed

CCD100: test mk3 protocol file with mk2 device #7005

FreddieAkeroyd opened this issue Feb 6, 2022 · 13 comments
Assignees

Comments

@FreddieAkeroyd
Copy link
Member

As a developer I would like to know if the mk3 (tcp) CCD100 protocol file added during #6440 can be merged/adapted for use with an existing mk2 (serial) CCD100

acceptance criteria

  • mk2 device tested
  • files merged or macro added to select device to use
@esmith1729 esmith1729 self-assigned this May 16, 2023
@esmith1729 esmith1729 added the 0 label May 16, 2023
@esmith1729 esmith1729 added this to the SPRINT_2023_05_11 milestone May 16, 2023
@github-actions github-actions bot added the ready label May 16, 2023
@esmith1729
Copy link
Contributor

Doesn't look possible as it requires substituting an InTerminator at runtime (mk3 uses "\r\n" but mk2 uses "\r\r\n"), as well as uncommenting a line from getCurrRead.

@github-actions github-actions bot added review and removed ready labels May 16, 2023
@KathrynBaker
Copy link
Member

Terminators can be set using in the st.cmd file, alternatively different protocol files could be opened based on the model number, slightly more complicated, but possible using a macro that is then compared in the st.cmd file (we do this for Danfysiks as an example)

@rerpha
Copy link
Contributor

rerpha commented May 16, 2023

I don't think you can set system vars when referencing a .proto file from a db - we tried setting the terminator based on $(IFMK3) and then passing as $2 of each .proto() call but it didn't work.

I also don't think we'd be able to uncomment the in line which is required for the mk2 but not mk3

@Tom-Willemsen
Copy link
Contributor

You don't have to set the terminators at the streamdevice level, it can be done at the asyn level instead.

See https://github.com/ISISComputingGroup/EPICS-ioc/blob/7a9a8b3bedfd769d10797b4e1642500763c427f7/KHLY2400/iocBoot/iocKHLY2400-IOC-01/st-common.cmd#L18 for an example of how we do variable terminators for a keithley 2400 for example.

If it's just one protocol function that's different, we might just need to have read_mk2 and read_mk3 functions - or have different protocol fragments with the same function names. There might still be a small amount of duplication but at least not a full .proto file.

Give me a shout on teams if this doesn't make sense and I will explain better...

@rerpha
Copy link
Contributor

rerpha commented May 17, 2023

protocol file for a keithley 2400 (note no terminators in this file as tom says they are set at asyn level) https://github.com/ISISComputingGroup/EPICS-Keithley2400/blob/master/Keithley2400Sup/Keithley2400.proto

@esmith1729
Copy link
Contributor

https://github.com/ISISComputingGroup/EPICS-asyn/blob/master/asyn/miscellaneous/asynInterposeEos.c#L328-L336

Asyn doesn't let you use more than 2 characters for the Eos characters it seems?

@rerpha
Copy link
Contributor

rerpha commented May 23, 2023

Evan and I have modified it so it works with 3 chars: ISISComputingGroup/EPICS-asyn#25

I'm not sure if it was set to a 2 char limit for a good reason but if not it might be worth pushing this upstream?

@rerpha
Copy link
Contributor

rerpha commented Jul 26, 2023

Works well with MK3 - just tested one CG set up
One of the ioc tests to do with logging is failing, so still investigating. I think it is the mismatch handlers which prevent asyn from logging to the file immediately - @FreddieAkeroyd do you think this is an unintended side effect? would think they would eventually time out and log to say no response? if it's fine (which I think it is, things will still be in alarm to let us know that comms aren't working?) we should just remove the tests i think.

@rerpha
Copy link
Contributor

rerpha commented Sep 6, 2023

decided to remove the tests, as there's no point testing against the prevention of something if it is no longer an issue. we don't test logging for other IOCs either.

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

No branches or pull requests

6 participants