Skip to content

Conversation

@gsulc
Copy link
Contributor

@gsulc gsulc commented Nov 26, 2019

Some small changes to p402.py to make it a little easier to understand in preparation for solving issues #167 and #169 . I also improved some log statements.

I have two open questions about setup_402_state_machine(): why is it necessary to enter the "Pre-Operational" NMT state to read the pdo configurations. And why change state to "Switch on Disabled"? Shouldn't the present state be kept?

Some small changes to p402.py to make it a little easier to understand. I have two open questions about setup_402_state_machine(): why is it necessary to enter the "Pre-Operational" NMT state to read the pdo configurations. And why change state to "Switch on Disabled"? Shouldn't the present state be kept?
@gsulc gsulc marked this pull request as ready for review November 27, 2019 03:45
@af-silva
Copy link
Collaborator

Sorry @gsulc, I'm with some work right now... I will try to review your changes by the end of the week.
But a quick peek at the changes seems everything is ok, I just notice a small thing in the home function, the way you set the return of the function (return right the value instead of at the end of execution) bypass the finally statement of the try-catch when returning true...

But during the weekend I will review and discuss with you.

@gsulc
Copy link
Contributor Author

gsulc commented Nov 27, 2019

I understand. Get to it when you can. The return statement will execute after the finally clause. From the Python documentation:

If the try statement reaches a break, continue or return statement, the finally clause will execute just prior to the break, continue or return statement’s execution.

It's pretty standard in languages, though I didn't know it for a long time either.

Regarding the op_mode setter: why always set self.state = start_state at the end?

@gsulc
Copy link
Contributor Author

gsulc commented Dec 8, 2019

I have solutions for those tickets. If you want, I can resolve the conflicts and merge, then submit those changes. I don't want to step on people's toes though.

@af-silva
Copy link
Collaborator

af-silva commented Dec 9, 2019

The return statement will execute after the finally clause. From the Python documentation:

You are absolutely right, my bad.

Today I will review and merge your changes with the code base. Keep in mind that I'm trusting the fixes are not breaking anything, at the moment I can't validate this with real hardware.

I have solutions for those tickets

What thickets are you referring to?

I don't want to step on people's toes though

You are not stepping in anyone toes @gsulc, I'm happy to see people contributing code to this project.

af-silva added a commit that referenced this pull request Dec 10, 2019
@af-silva af-silva merged commit e92a4e4 into canopen-python:master Dec 10, 2019
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.

2 participants