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 OrangePI R1 detection #23

Merged
merged 4 commits into from
May 19, 2019
Merged

Conversation

kouliss
Copy link
Contributor

@kouliss kouliss commented May 16, 2019

No description provided.

@@ -336,6 +337,14 @@ def _armbian_id(self):
return ORANGE_PI_PC
return None

def _armbian_id(self):
Copy link
Member

Choose a reason for hiding this comment

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

this code should be merged with the above, not replace!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean like this?

def _armbian_id(self):
"""Check whether the current board is an OrangePi PC."""
board_value = self.detector.get_armbian_release_field('BOARD')
if board_value == "orangepipc":
return ORANGE_PI_PC
if board_value == "orangepi-r1":
return ORANGE_PI_R1
return None

Copy link
Member

Choose a reason for hiding this comment

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

yep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, changed

Copy link
Contributor

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Please fix the Travis errors. It looks like there are some tabs instead of spaces for a few lines and they weren't lining up properly. Thanks.

@makermelissa
Copy link
Contributor

makermelissa commented May 19, 2019

I noticed a couple more things that need to be added.

We'll need an Orange Pi IDs list like

_ORANGE_PI_IDS = (
    ORANGE_PI_PC,
    ORANGE_PI_R1
)

Also we'll need the any_orange_pi property function modified to look in the list with something like:

    @property
    def any_orange_pi(self):
        """Check whether the current board is any defined Orange Pi."""
        return self.id in _ORANGE_PI_IDS

@kouliss
Copy link
Contributor Author

kouliss commented May 19, 2019

changed and updated

Copy link
Contributor

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Looks like there's a couple more changes. Please see comments.

@@ -25,8 +25,6 @@
GENERIC_LINUX_PC = "GENERIC_LINUX_PC"
PYBOARD = "PYBOARD"
NODEMCU = "NODEMCU"
ORANGE_PI_PC = "ORANGE_PI_PC"
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll want to leave both of these IDs here too. This is where they are defined.

@@ -384,7 +388,7 @@ def any_beaglebone(self):
@property
def any_orange_pi(self):
"""Check whether the current board is any defined Orange Pi."""
return self.ORANGE_PI_PC
return self.ORANGE_PI_IDS
Copy link
Contributor

Choose a reason for hiding this comment

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

This should return a True/False value instead of a list. Also, the list starts with an underscore character.

@makermelissa
Copy link
Contributor

At a glance, this looks good. I'll test it.

Copy link
Contributor

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Tested good.

@makermelissa
Copy link
Contributor

@ladyada, looks like you'll need to merge this in. I don't have the permissions currently.

@ladyada
Copy link
Member

ladyada commented May 19, 2019

ok cool to verify you got an R1?

@makermelissa
Copy link
Contributor

Yes I did.

@ladyada ladyada merged commit 72bd91f into adafruit:master May 19, 2019
@ladyada
Copy link
Member

ladyada commented May 19, 2019

rad, ill give you perms too

@makermelissa
Copy link
Contributor

Thanks :)

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.

3 participants