Skip to content

Accept unit classes in motor groups#480

Merged
WasabiFan merged 7 commits intoev3dev-stretchfrom
motor-group-units
Jul 29, 2018
Merged

Accept unit classes in motor groups#480
WasabiFan merged 7 commits intoev3dev-stretchfrom
motor-group-units

Conversation

@WasabiFan
Copy link
Copy Markdown
Member

I did some things... primarily enabling unit classes in motor groups, plus some other fixes. Also created tests for these things because I don't trust my programming abilities (nor Python).

Targeting this PR on the docs branch because it has other changes to these same files which I don't want to deal with as merge conflicts.

Fixes #474

@WasabiFan WasabiFan requested review from ddemidov and dwalton76 June 29, 2018 03:38
@WasabiFan
Copy link
Copy Markdown
Member Author

I don't know why that CI error is happening... clearly the files aren't opened as r/w but I don't see why that is different there.

@ddemidov
Copy link
Copy Markdown
Member

It looks like the first failure is at api_tests.py:144, and the specific error is io.UnsupportedOperation: File not open for writing.

@ddemidov
Copy link
Copy Markdown
Member

ddemidov commented Jun 29, 2018

Could it be the file permissions are read-only in fake-sys repo?

EDIT: nope, those seem to be ok. I am out of ideas for now.

@WasabiFan
Copy link
Copy Markdown
Member Author

The CI issue should be fixed by #481.

@ddemidov
Copy link
Copy Markdown
Member

The CI issue should be fixed by #481.

Can you rebase this on stretch then?

@WasabiFan WasabiFan force-pushed the stretch-documentation branch from 5e89179 to 192df65 Compare July 14, 2018 02:42
@WasabiFan WasabiFan force-pushed the motor-group-units branch from 83e7066 to 09c0f8a Compare July 14, 2018 02:48
@WasabiFan
Copy link
Copy Markdown
Member Author

I've rebased both PRs. I think it probably makes the most sense to merge #411 before this one, just to keep the commits somewhat logical. That PR is complete aside from the fact that I'd like to add some info about how to use the new features; I'd appreciate a review of the content over there as well.

@WasabiFan WasabiFan force-pushed the motor-group-units branch from ed3111c to aa3c12e Compare July 17, 2018 01:59
@WasabiFan WasabiFan changed the base branch from stretch-documentation to stretch July 17, 2018 01:59
@WasabiFan WasabiFan changed the base branch from stretch to ev3dev-stretch July 20, 2018 02:18
@WasabiFan
Copy link
Copy Markdown
Member Author

@ddemidov Could you check over this and #483? Once I merge these two PRs I plan to test all the basic functions on my EV3 and then publish a release -- my intent is to do that within the next few days if at all possible.

Copy link
Copy Markdown
Member

@ddemidov ddemidov left a comment

Choose a reason for hiding this comment

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

I was out of the loop on this set of changes, and I did not test if it works on the ev3, so I think I'll just let you decide if this is ready to merge.

Some specific things that caught my eye is old-style formatting ('%s' % (v) instead of '{}'.format(v)), but I am not sure how strict are we about these kinds of things.

Also, print is used to spew warnings as opposed to logging module elswhere. We should probably decide on one of those.

Copy link
Copy Markdown
Member

@ddemidov ddemidov left a comment

Choose a reason for hiding this comment

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

This looks good to me (I assume @dlech has already checked debian-related changes).

@WasabiFan WasabiFan merged commit caa01ad into ev3dev-stretch Jul 29, 2018
@WasabiFan WasabiFan deleted the motor-group-units branch November 10, 2018 00:26
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.

Accept SpeedInteger classes in motor groups

2 participants