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

Pps with modulations dev #28

Merged
merged 9 commits into from
Aug 26, 2016
Merged

Conversation

alecive
Copy link
Member

@alecive alecive commented Aug 25, 2016


  

Sorry, something went wrong.

@@ -11,6 +11,8 @@
#define SKIN_THRES 7 // Threshold with which a contact is detected
#define RESP_GAIN_FOR_SKINGUI 100 //To amplify PPS activations from <0,1> to <0,100>
#define PPS_AGGREG_ACT_THRESHOLD 0.2 //Threshold for aggregated events per skin part
#define NR_ARM_JOINTS 7
#define NR_TORSO_JOINTS 3
Copy link
Member Author

Choose a reason for hiding this comment

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

Is it really needed to hard code a macro for the number of torso and arm joints?

Torso joints are already read from the encoder and stored in the jntsT variable. For the arm, you cannot read it from the encoders, but you can have a similar interface, i.e. a jntsAL variable that you initialize to 7 as a member of the class, instead of a #define. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Matter of philosophy - macros make it IMHO more transparent.

Copy link
Member Author

Choose a reason for hiding this comment

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

I know that, but since everything else is handled with another philosophy, it's better to stick to the existing one :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer having subVector(0,NR_ARM_JOINTS -1) over subVector(0,6). If you object against macros, it can be made a variable with the same name (UPPER_CASE) and initialized in init(). It's slightly more work, but is the same. What is the problem with macro?

Copy link
Member Author

@alecive alecive Aug 25, 2016

Choose a reason for hiding this comment

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

The problem is that the other code uses jntsT jntsR, etc, so I would stick with that. Tha is I would use a variable (called, to be consistent, jntsAR jntsAL), initialized in the init as you said (this is what I suggested in the beginning).

jntsT is already initialized and used, so you can use that for the torso.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, no problem.

  • Torso joints solved by this commit: 0fe466a
  • (Sorry about this commit: 0fe466a - I forgot to pull first, then rebase somehow did not work out and I had to merge manually with your commit with the formatting edits)
  • Arm joints solved by this commit 67c7c6f

@matejhof
Copy link
Collaborator

Just tested everything - torso joints are taken into account now for remapping to taxel frames. Projections of taxel positions to camera frames are still operational too.

@matejhof matejhof merged commit 6f15741 into pps-with-modulations Aug 26, 2016
@matejhof matejhof deleted the pps-with-modulations-dev branch August 26, 2016 09:33
@alecive
Copy link
Member Author

alecive commented Aug 26, 2016

Good!

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.

None yet

2 participants