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

Added sequence tests #27

Merged
merged 11 commits into from
Aug 3, 2018
Merged

Conversation

ericlesaquiles
Copy link
Collaborator

No description provided.

@k-okada k-okada requested a review from Affonso-Gui July 18, 2018 01:36
@@ -1024,10 +1024,10 @@ the condition to go uncaught if it cannot be classified."
"Create a test that FORM, which should produce a fresh value,
does not improperly introduce sharing during constant folding."
`(deftest ,name
(flet ((%f () (declare (optimize (speed 3) (safety 0) (space 0)
Copy link
Member

Choose a reason for hiding this comment

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

@Affonso-Gui
Copy link
Member

Are you using euslisp from source or from apt?
Bit vectors should have been fixed in euslisp/EusLisp#281, make sure you have it to run bit-vector tests.

Also, it appears that the latest euslisp requires building roseus from source too in Ubuntu 16 euslisp/EusLisp#291. Let us know if you have any problems with that.

@ericlesaquiles
Copy link
Collaborator Author

I thought I was using the latest version (roseus was 1.7, if I'm not mistaken), but it might've slipped. Now, is it just me or there is no more roseus from apt (sudo apt install ros-kinetic-roseus doesn't work anymore)?

Anyway, I'm using euslisp from source now. But, after uninstalling roseus, I've been unable to bring it back (installation fails, mostly because it can't find euslisp's ros configuration files, which were removed prior to installing from source.. I've tried some changes on roseus's installation files to try to circumvent that, but none worked).

@ericlesaquiles
Copy link
Collaborator Author

ericlesaquiles commented Jul 20, 2018

Now, with euslisp from source, when running the tests I'm having unbound variable mtimer. Do you have some idea on what that might be? I have found no reference to mtimer either on euslisp's source code or on the manual..

@Affonso-Gui
Copy link
Member

I was able to run from source with the following:

git clone https://github.com/euslisp/jskeus.git
cd jskeus
make
source bashrc.eus

cd /path/to/workspace/src
git clone https://github.com/jsk-ros-pkg/jsk_roseus.git

>> change $(euslisp_INCLUDE_DIRS) to the path to jskeus/eus/include in include_directories() line of jsk_roseus/roseus/CMakeLists.txt

cd /path/to/workspace
catkin build roseus
source devel/setup.bash

@Affonso-Gui
Copy link
Member

I get no errors at mtimer, but the test stops at peek-char.1. We might need to have a look at that.

@ericlesaquiles
Copy link
Collaborator Author

I did the changes you mentioned (even when probably not needed):

if(APPLE)
-  if(NOT EXISTS /usr/local/opt/jskeus/eus)  # $EUSDIR in OS X
+  if(NOT EXISTS /home/ericles/Projetos/JSK/jsk-projects/jskeus/eus)  # $EUSDIR in OS X
     message(FATAL_ERROR "jskeus is not installed via Homebrew, Please run `brew install homebrew/x11/jskeus`")
   endif()
-  set(euslisp_INCLUDE_DIRS /usr/local/opt/jskeus/eus/include)
+  set(euslisp_INCLUDE_DIRS /home/ericles/Projetos/JSK/jsk-projects/jskeus/eus/include)
 else()
   find_package(euslisp REQUIRED)
   find_package(jskeus REQUIRED)
@@ -60,11 +60,11 @@ else()
       set(euslisp_PACKAGE_PATH ${euslisp_PREFIX}/share/euslisp)
     endif()
     message("-- Set euslisp_PACKAGE_PATH to ${euslisp_PACKAGE_PATH}")
-    set(euslisp_INCLUDE_DIRS ${euslisp_PACKAGE_PATH}/jskeus/eus/include)
+    set(euslisp_INCLUDE_DIRS /home/ericles/Projetos/JSK/jsk-projects/jskeus/eus/include)
   endif()
 endif()
-message("-- Set euslisp_INCLUDE_DIRS to ${euslisp_INCLUDE_DIRS}")
-include_directories(/usr/include /usr/X11R6/include ${euslisp_INCLUDE_DIRS} ${catkin_INCLUDE_DIRS})
+message("-- Set euslisp_INCLUDE_DIRS to /home/ericles/Projetos/JSK/jsk-projects/jskeus/eus/include")
+include_directories(/usr/include /usr/X11R6/include /home/ericles/Projetos/JSK/jsk-projects/jskeus/eus/include ${catkin_INCLUDE_DIRS})
 add_library(roseus roseus.cpp)
 add_library(eustf eustf.cpp)
 add_library(roseus_c_util roseus_c_util.c)

Even so on trying to install roseus, I got the following:

➜  kinetic sdo catkin build roseus
--------------------------------------------------------------
Profile:                     default
Extending:          [cached] /opt/ros/kinetic
Workspace:                   /home/ericles/ros/kinetic
--------------------------------------------------------------
Source Space:       [exists] /home/ericles/ros/kinetic/src
Log Space:          [exists] /home/ericles/ros/kinetic/logs
Build Space:        [exists] /home/ericles/ros/kinetic/build
Devel Space:        [exists] /home/ericles/ros/kinetic/devel
Install Space:      [unused] /home/ericles/ros/kinetic/install
DESTDIR:            [unused] None
--------------------------------------------------------------
Devel Space Layout:          linked
Install Space Layout:        None
--------------------------------------------------------------
Additional CMake Args:       None
Additional Make Args:        None
Additional catkin Make Args: None
Internal Make Job Server:    True
Cache Job Environments:      False
--------------------------------------------------------------
Whitelisted Packages:        None
Blacklisted Packages:        None
--------------------------------------------------------------
Workspace configuration appears valid.
--------------------------------------------------------------
[build] Found '9' packages in 0.0 seconds.                                                                                                            
[build] Package table is up to date.                                                                                                                  
Starting  >>> roseus                                                                                                                                  
______________________________________________________________________________________________________________________________________________________
Errors     << roseus:cmake /home/ericles/ros/kinetic/logs/roseus/build.cmake.016.log                                                                  
CMake Error at /home/ericles/ros/kinetic/src/jsk_roseus/roseus/CMakeLists.txt:54 (find_package):
  By not providing "Findeuslisp.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "euslisp", but
  CMake did not find one.

  Could not find a package configuration file provided by "euslisp" with any
  of the following names:

    euslispConfig.cmake
    euslisp-config.cmake

  Add the installation prefix of "euslisp" to CMAKE_PREFIX_PATH or set
  "euslisp_DIR" to a directory containing one of the above files.  If
  "euslisp" provides a separate development package or SDK, be sure it has
  been installed.


cd /home/ericles/ros/kinetic/build/roseus; catkin build --get-env roseus | catkin env -si  /usr/bin/cmake /home/ericles/ros/kinetic/src/jsk_roseus/rose
us --no-warn-unused-cli -DCATKIN_DEVEL_PREFIX=/home/ericles/ros/kinetic/devel/.private/roseus -DCMAKE_INSTALL_PREFIX=/home/ericles/ros/kinetic/install;
 cd -
......................................................................................................................................................
Failed     << roseus:cmake          [ Exited with code 1 ]                                                                                            
Failed    <<< roseus                [ 0.8 seconds ]                                                                                                   
[build] Summary: 0 of 1 packages succeeded.                                                                                                           
[build]   Ignored:   8 packages were skipped or are blacklisted.                                                                                      
[build]   Warnings:  None.                                                                                                                            
[build]   Abandoned: None.                                                                                                                            
[build]   Failed:    1 packages failed.                                                                                                               
[build] Runtime: 0.9 seconds total.       

Do we have a Cmake file for euslisp? it seems to want to look at one of those. Or a different CMAKE_PREFIX_PATH, but I'm not sure what to do about it as yet
(btw, I did previously install euslisp on the jskeus folder aforementioned)..

Apart from that, new strange errors are popping up when running the tests with eus. The mtimer error has strangely gone for now, but then I got undefined function with-gensyms. I looked it up, and that macro really seems to be undefined (was it removed? I couldn't find its mention on recent commits though), so I defined it.. to get other stranger errors. I wonder whether there is some problem with the installation (I guess not, as it ran smoothly, with no apparent error messages), or whether the behavior with roseus is expected to be different on this matter (I guess not as well, but this is just guessing).

Maybe those errors are an expected result of recent changes.

@Affonso-Gui
Copy link
Member

Affonso-Gui commented Jul 22, 2018

About the installation:

It seems that your build cannot locate the euslisp package (try to roscd euslisp). Since we are including it from source in the include_directories it should not be necessary, but it still remains listed as a dependency in the CMake, which is causing the errors.

You can try to:

  • apt-install and source euslisp (that is my case).
    or
  • Fix the roseus CMake to do not depend on euslisp package anymore

About the tests:

The unittest makes uses of resources which are only available at roseus, such as mtimer, with-gensyms, and possibly others. At this point of time eus-test.l is only supported by roseus (and maybe irteusgl, I am not sure).

Maybe we should fix this too, since the test suite is designed for pure euslisp, without ros utilities.

EDIT: tests stopping after peek-char.1 is addressed on #29

@ericlesaquiles
Copy link
Collaborator Author

Just having apt-installed euslisp (and jskeus) is not enough (it was the first thing I tried, incidentally). Now, it goes too few arguments to function ‘cell*(...).. I'm trying to figure it out

About the tests, I think it is not a problem to trust in some of the roseus-only features, insofar as they're not ros-related features (it raises the question though.. if they're not ros related, why are they on roseus?). It seems enough for me to be aware we are making use of those features and to know which features they are (the second part could, perhaps, be better addressed closer to the end).

About the peek.. I'll try to adress it shortly (some of the others as well).

By the way, about the infix notation, did you consider #y (as in "ynfix")? it sound good to me

@Affonso-Gui
Copy link
Member

Affonso-Gui commented Jul 22, 2018

Please post the complete result of catkin build roseus here so we can debug.

if they're not ros related, why are they on roseus?

Actually they seem to defined in the irt extension. Just tried, and appears that the tests also run well in irteusgl.

By the way, about the infix notation, did you consider #y (as in "ynfix")?

Seems good to me too. I am however also thinking in the possibility of leaving the infix macro on the lib/ (without loading it by default).

@ericlesaquiles
Copy link
Collaborator Author

It is in the issue 30

Actually they seem to defined in the irt extension. Just tried, and appears that the tests also run well in irteusgl.

Yes, (luckily for me) that seems to be the case

I am however also thinking in the possibility of leaving the infix macro on the lib/ (without loading it by default).

That's a solution, but if people are not using it much right now, I wonder if making them load it by hand wouldn't make them even less willing to use it. Not that it would make a big difference, though

@ericlesaquiles
Copy link
Collaborator Author

Are you not having segmentation fault as of now, @Affonso-Gui ?

@k-okada
Copy link
Member

k-okada commented Jul 23, 2018

it compile euslisp/jskeus within catkin workspace, we need to add files manually.
For esulisp download CMakeLists.txt, package,xml, cmake/ and env-hook/ from https://github.com/tork-a/euslisp-release.
For jskeus, download CMakeLists.txt and package.xml from https://github.com/tork-a/jskeus-release

you can also check the script https://github.com/jsk-ros-pkg/jsk_roseus/blob/master/setup_upstream.sh

Copy link
Member

@Affonso-Gui Affonso-Gui left a comment

Choose a reason for hiding this comment

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

  • Restore commented-out parts in search-aux.lsp using bit-vectors and eus-like subseq
  • The following auxiliary functions do not behave as expected. Please test your code before sending PRs.
equalp
substitute
substitute-if
substitute-if-not
nsubstitute
nsubstitute-if
nsubstitute-if-not
mismatch
peek-char



;; Shadows peek-char to accept peek-type, and ignore it for now
(defun peek-char (&optional (peek-type nil) (stream *standard-input*)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It shouldn't work.. I've accidentally pushed the wrong update, but will push the correct one as soon as I can (I'm without my computer right now)..

@ericlesaquiles
Copy link
Collaborator Author

I have no excuse for the errors on those functions, but they should hopefully be fine now.
Strangely, bit vectors still yield many errors

@Affonso-Gui Affonso-Gui merged commit efaedce into euslisp:gsoc2018 Aug 3, 2018
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