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

Linux refactor: Episode I #2678

Merged

Conversation

Williangalvani
Copy link
Member

@Williangalvani Williangalvani commented Jun 11, 2024

splitting it up to make the review process smoother

tested on pi4/bullseye
test on pi5/bookworm
port mapping on pi4/bullseye has changed =[
updated serial mapping for pi4 on bullseye

@Williangalvani Williangalvani force-pushed the refactor_linux_part_1 branch 17 times, most recently from 8564fd6 to 6054c63 Compare June 12, 2024 01:12
@Williangalvani Williangalvani marked this pull request as ready for review June 12, 2024 01:19
Copy link
Member

@patrickelectric patrickelectric left a comment

Choose a reason for hiding this comment

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

Nice approach, just some minors

release = "Bullseye"
os_release = load_file("/etc/os-release")
if "bookworm" in os_release:
release = "Bookworm"
Copy link
Member

Choose a reason for hiding this comment

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

Can we use an enum here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

can this wait?
I plan to move the checks for os_release and pi version to commonwealth in the next PR

@joaoantoniocardoso
Copy link
Member

Nice one, eager for the next episodes!

@Williangalvani Williangalvani force-pushed the refactor_linux_part_1 branch 3 times, most recently from 5ed424a to 2805f3b Compare June 13, 2024 02:13
Copy link
Member

@patrickelectric patrickelectric left a comment

Choose a reason for hiding this comment

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

Should we merge this before 1.3 ? I'm more inclined to merge it in 1.4, but I'm open to discuss.

@joaoantoniocardoso
Copy link
Member

Should we merge this before 1.3 ? I'm more inclined to merge it in 1.4, but I'm open to discuss.

Yeah, might be a smarted move if this is not required for 1.3

@Williangalvani
Copy link
Member Author

Should we merge this before 1.3 ? I'm more inclined to merge it in 1.4, but I'm open to discuss.

Yes, it is probably wiser to keep it for the next sable

@patrickelectric patrickelectric merged commit 28edf42 into bluerobotics:master Aug 26, 2024
6 checks passed
@patrickelectric patrickelectric deleted the refactor_linux_part_1 branch August 26, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-after-stable Should be merged only after next stable release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants