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

Call a method to define min/max table in :init-ending. #428

Closed
wants to merge 1 commit into from

Conversation

snozawa
Copy link
Contributor

@snozawa snozawa commented Apr 18, 2017

Call a method to define min/max table in :init-ending because joint min/max table is jskeus original functionality.
For robots not requiring join min/max table, do nothing and do not change default behavior.
For robots requiring joint min/max table, please define :define-min-max-table method and do not need to call the method explicitly in subclass.

C.f.
start-jsk/rtmros_tutorials#508
start-jsk/rtmros_tutorials#509

…ding. By default, do nothing and do not change behavior for non-join-min-max robots.
@snozawa
Copy link
Contributor Author

snozawa commented Apr 18, 2017

Please review this.
@YoheiKakiuchi

snozawa added a commit to snozawa/rtmros_tutorials that referenced this pull request Apr 18, 2017
…ot define :init-ending if robot-model's :define-min-max-table exists (euslisp/jskeus#428). This will fix the bug (start-jsk#508) because unnecessary :init-ending conflict is resolved.
@YoheiKakiuchi
Copy link
Member

LGTM

@k-okada
Copy link
Member

k-okada commented Apr 18, 2017 via email

@k-okada
Copy link
Member

k-okada commented Apr 18, 2017 via email

@snozawa
Copy link
Contributor Author

snozawa commented Apr 18, 2017

@YoheiKakiuchi , @k-okada

Thank you for your confirmation and comments.

I also thought min-max-table could move to :init-ending

I think min-max table is already added to :init-ending by this PR.

  1. We also need :make-min-max-table thing , otherwise no one knows how to use this.
  2. We'd better to find robots that requires this feature other than hrp2/jaxon. So more people feels this is important feature.

I see.
I'll try to create an example to make and add min-max table in irteus/demo.
(may be related with https://github.com/euslisp/jskeus/blob/master/irteus/test/joint.l test program)

@snozawa
Copy link
Contributor Author

snozawa commented Apr 18, 2017

For #428 (comment).

My comment was wrong, we already have make-min-max-table, so why not use this. or override this.

:make-min-max-table is already used for hrp2/jaxon, such as,
https://github.com/start-jsk/rtmros_tutorials/blob/master/hrpsys_ros_bridge_tutorials/euslisp/make-joint-min-max-table.l#L22

From rbrain ages, :make-min-max-table is called in model generation phase, not in :init-ending.
This is because :make-min-max-table takes time.
Therefore, currently

For other issues:
#195 and #194 are still not solved, as you said.

#9 is already solved because we do not use hash-table for min-max table.
This is because of loadbility, modurality, and fast access.
#77 (comment)
#77 (comment)

@k-okada
Copy link
Member

k-okada commented Apr 18, 2017

so, how about this?

   (format f "(defmethod ~A~%" (send (class robot) :name))
   (format f "  (:init-ending~%")
   (format f "    ()~%")
   (format f "    (prog1~%")
   (format f "      (send-super :init-ending)~%")
   (labels
       ((gen-string-name
         (j)
         (if (stringp (send j :name)) (format nil "\"~A\"" (send j :name)) (send j :name))))
     (dolist (j (remove-if-not #'(lambda (j) (send j :joint-min-max-table)) (send robot :joint-list)))
       (format f "      (send (send self :joint ~A) :joint-min-max-table~%" (gen-string-name j))
       (format f "                              '~A~%" (send j :joint-min-max-table))
       (format f "                              )~%")
       (format f "      (send (send self :joint ~A) :joint-min-max-target (send self :joint ~A))~%"
               (gen-string-name j) (gen-string-name (send j :joint-min-max-target)))
       )
     (format f "      ))~%")
     ))

@snozawa
Copy link
Contributor Author

snozawa commented Apr 18, 2017

I see.

Suggested approach (#428 (comment)) is similar to the current version of rtmros_tutorials in terms of start-jsk/rtmros_tutorials#508.

My motivation of this PR is to remove :init-ending definition in robot's model file (such as hrp2jsk.l),
because some robots has :init-ending overwrite in robot utils files.
And, this cause some problem:start-jsk/rtmros_tutorials#508.

The downstream PR start-jsk/rtmros_tutorials#509 can also fix the problem (start-jsk/rtmros_tutorials#508) without using this PR.

If #428 (comment) is better than this PR (adding :define-min-max-table in robot-model's init-ending),
I can close this PR and i can fix the problem through start-jsk/rtmros_tutorials#509.

@k-okada
Copy link
Member

k-okada commented Apr 18, 2017

Suggested approach (#428 (comment)) is similar to the current version of rtmros_tutorials in terms of start-jsk/rtmros_tutorials#508.

but current version has chance to forget to call define-min-max-table, but my proposal approach is not

My motivation of this PR is to remove :init-ending definition in robot's model file (such as hrp2jsk.l),

It seems current min-max-table is designed to set table when you make each joint, https://github.com/euslisp/jskeus/blob/1.1.0/irteus/irtmodel.l#L44
so best way is to set min-max-table when we make each joint,

@snozawa
Copy link
Contributor Author

snozawa commented Apr 18, 2017

@snozawa snozawa closed this Apr 18, 2017
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