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

SmartGCal support for Flamingos2 #1573

Merged
merged 3 commits into from
Jan 22, 2025
Merged

Conversation

cquiroz
Copy link
Contributor

@cquiroz cquiroz commented Jan 21, 2025

This is for review I still need to do a core release before merging.
@swalker2m This is what I found was needed for SmartGCal but maybe I'm missing something else?


object Flamingos2:

case class SearchKey(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is taken from Gmos, I'm not sure if I need a separate SearchKey and TableKey

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm struggling to remember why I thought I needed distinct types for GMOS. It may be that the search key once separated the wavelength. I'm not sure. If you don't see a need for the distinction by all means merge them.

PRIMARY KEY (c_instrument, c_gcal_id),

FOREIGN KEY (c_instrument, c_gcal_id) REFERENCES t_gcal(c_instrument, c_gcal_id),
CONSTRAINT check_is_gmos_south CHECK (c_instrument = 'Flamingos2'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_is_f2

@swalker2m
Copy link
Contributor

maybe I'm missing something else?

We'll need to update the lucuma.odb.logic.SmartGcalImplementation class when it is time to do sequence generation, but this looks great so far.

@cquiroz cquiroz force-pushed the sc-4541-add-smartgcal-support-for-f2-ls branch from 14cf165 to 6d1971c Compare January 21, 2025 21:58
Copy link
Contributor

@toddburnside toddburnside left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, as far as I can tell.

@cquiroz cquiroz merged commit 094af37 into main Jan 22, 2025
6 checks passed
@cquiroz cquiroz deleted the sc-4541-add-smartgcal-support-for-f2-ls branch January 22, 2025 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants