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.

Sign up for GitHub

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

Jump to bottom

Fix/18.04 #241

Merged
merged 10 commits into from
Feb 3, 2020
Merged

Fix/18.04 #241

merged 10 commits into from
Feb 3, 2020

Conversation

floriantschopp
Copy link
Contributor

Make it compile on 18.04 (can someone on 16.04 test this please?)

Related issues are:

@vilvo
Copy link

vilvo commented Nov 13, 2018

I tested Kalibr in an Ubuntu 18.04 LXC container with the same change a few weeks back. Works fine.
One input from that test is that LXC runs in non-windowing environment so the PDF reports do not get generated. Would be nice to have them even without displaying the PDF results.

@andre-nguyen
Copy link

👍 This is needed for multicam calibration to work.

@HannesSommer
Update MulticamGraph.py
d8fc0e2 
Preserve some backward compatibility with older python versions.
@HannesSommer
Copy link
Contributor

Has somebody tested this on 16.04 in the meantime?

@helenol
Copy link
Collaborator

helenol commented Jan 17, 2020 •
edited
Loading

Can we merge this now that it's 2020? :) Jenkins says it builds on 16.04 in case anyone is still in the dark ages.

@floriantschopp
Copy link
Contributor Author

OK with me :-)

@HannesSommer
Copy link
Contributor

Apparently nobody feels really responsible for this anymore. I'd suggest to merge this without the add option to manually input forcal length, which doesn't fit this PR and has possibly bad implications for people using this as part of unsupervised scripts. Also, it has an indentation problem and a comment was not properly moved, but that we could address in a separate PR. I know it was an MR before into this branch but that is also bad for the history and I wouldn't want to accept that part in the current state.

@floriantschopp , do you have time to quickly split the two? Otherwise, I can do it, but that would obfuscate authorship.

@floriantschopp
Copy link
Contributor Author

@HannesSommer Will do tomorrow 👍

@floriantschopp
Revert "Merge pull request #304 from ethz-asl/feature/input_focal_len…
aac34c2 
…gth_guess"

This reverts commit 2999e18, reversing
changes made to ee2b09d.
@floriantschopp floriantschopp mentioned this pull request Feb 3, 2020
Merged
HannesSommer
HannesSommer approved these changes Feb 3, 2020
View reviewed changes
Copy link
Contributor

@HannesSommer HannesSommer left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM. We should not forget to squash the history.

@floriantschopp floriantschopp merged commit cb6fb66 into master Feb 3, 2020
@ruffsl
Copy link

ruffsl commented Feb 6, 2020

Speaking of building on Ubuntu 18.04, perhaps someone could checkout the new PR adding a Dockerfile for reproducible builds, as I'm not sure how to fix the includes sm_common package: #347 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrepfr andrepfr andrepfr left review comments

@HannesSommer HannesSommer HannesSommer approved these changes

@ZacharyTaylor ZacharyTaylor Awaiting requested review from ZacharyTaylor

@schneith schneith Awaiting requested review from schneith

@smauq smauq Awaiting requested review from smauq

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@floriantschopp @vilvo @andre-nguyen @HannesSommer @helenol @ruffsl @andrepfr