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

Add base AD PVs such as SDK, Firmware, Serial Number #1070

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kgofron
Copy link

@kgofron kgofron commented Sep 7, 2022

No description provided.

@tacaswell
Copy link
Contributor

Can you drop the first two commits from this branch and force-push to have just the third?

@kgofron
Copy link
Author

kgofron commented Sep 7, 2022

I dropped the two prior commits, and pushed the 3-rd commit.

@tacaswell
Copy link
Contributor

There are now 6 commits...

@danielballan
Copy link
Member

@kgofron git revert adds a commit, leaving a record that some previous commit has been un-done. Someone a merge commit ended in here too.

@tacaswell wants the original two commits dropped from history.

# Reset back to before the `git revert` commits were added.
git reset --hard 56fa1b3
git rebase -i HEAD~4 # Interactively delete your first two commits.
git push kgofron add_base_pv --force-with-lease

@kgofron
Copy link
Author

kgofron commented Sep 8, 2022

'--force-with-lease' does not work:

ophyd/ophyd/areadetector$ git push kaz add_base_pv --force-with-lease
To https://github.com/kgofron/ophyd
! [rejected] add_base_pv -> add_base_pv (stale info)
error: failed to push some refs to 'https://github.com/kgofron/ophyd'


Used '--force'
ophyd/ophyd/areadetector$ git push kaz add_base_pv --force
Enumerating objects: 9, done.
Counting objects: 100% (9/9), done.
Delta compression using up to 24 threads
Compressing objects: 100% (5/5), done.
Writing objects: 100% (5/5), 549 bytes | 549.00 KiB/s, done.
Total 5 (delta 4), reused 0 (delta 0)
remote: Resolving deltas: 100% (4/4), completed with 4 local objects.
To https://github.com/kgofron/ophyd

=======================
You could 'cherry-pick yourself'. https://git-scm.com/docs/git-cherry-pick

manufacturer = ADCpt(EpicsSignalRO, "Manufacturer_RBV")
model = ADCpt(EpicsSignalRO, "Model_RBV")
serial_number = ADCpt(EpicsSignalRO, "SerialNumber_RBV")
Copy link
Member

Choose a reason for hiding this comment

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

I believe these came in from AreaDetector R2-6: https://klauer.github.io/ad_compare/ (search for SerialNumber_RBV and check the first column that doesn't say 'no').

It's (probably) fine if ophyd starts assuming a newer version of AreaDetector than the ancient R1-9-1, but it should at least be a very prominent part of the release notes.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could read the Signal for ADCoreVersion_RBV and then conditionally include functionality based on that? Though I agree that assuming newer than R1-9-1 going forward is probably OK

@danielballan
Copy link
Member

@kgofron I suspect you did the merge on GH and so you need to either pull that merge commit you created on the remote first or else do plain --force. It is good to be fluent with the collaborative features of git. You can find very old public PRs of others helping me learn this, and I am happy to pay it forward.

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

Successfully merging this pull request may close these issues.

5 participants