acowley / roshask

Haskell client library for the ROS robotics framework.
BSD 3-Clause "New" or "Revised" License
107 stars 18 forks source link

roshask subscriber failing with "too few bytes. Failed reading at byte position 48" #15

Closed rgleichman closed 10 years ago

rgleichman commented 10 years ago

While running a roshask node that subscribes to a topic, instead of printing out the message it received, it prints out SensorSubscriber.hs: too few bytes. Failed reading at byte position 48. This happens as the topic starts publishing. I think this only occurs when reading the message.

To duplicate:

  1. start roscore
  2. start the roshask node runhaskell SensorSubscriber.hs
  3. run rostopic pub -r 1 optical/right pr2_pretouch_sensor_optical/OpticalBeams '{data: [100,100,100,100], broken: [true, true, false, true]}'
  4. The roshask node prints SensorSubscriber.hs: too few bytes. Failed reading at byte position 48

If during step 3 instead rostopic pub -r 1 optical/right pr2_pretouch_sensor_optical/OpticalBeams '{broken: [true, true, false, true]}' is run (different message data), then the roshask node prints SensorSubscriber.hs: too few bytes. Failed reading at byte position 40

The message type is pr2_pretouch_sensor_optical/OpticalBeams, below is included the roshask program, msg file, and the Haskell message type.

roshask program SensorSubscriber.hs

module SensorSubscriber (main) where

import Ros.Node (subscribe)
import Ros.Node (runHandler)
import Ros.Node (runNode)
import Ros.Pr2_pretouch_sensor_optical.OpticalBeams

rightTopicName :: String
rightTopicName = "optical/right"

leftTopicName :: String
leftTopicName = "optical/left"

showMsg :: OpticalBeams -> IO ()
showMsg  = putStrLn . ("I heard ya" ++) . show

main :: IO ()
main = runNode "SensorSubscriber" $ runHandler showMsg =<< subscribe rightTopicName

pr2_pretouch_sensor_optical/OpticalBeams msg

Header header
uint16[] data
bool[] broken

pr2_pretouch_sensor_optical/OpticalBeams Haskell type

{-# LANGUAGE OverloadedStrings, DeriveDataTypeable #-}
module Ros.Pr2_pretouch_sensor_optical.OpticalBeams where
import qualified Prelude as P
import Prelude ((.), (+), (*))
import qualified Data.Typeable as T
import Control.Applicative
import Ros.Internal.RosBinary
import Ros.Internal.Msg.MsgInfo
import Ros.Internal.Msg.HeaderSupport
import qualified Data.Vector.Storable as V
import qualified Data.Word as Word
import qualified Ros.Std_msgs.Header as Header

data OpticalBeams = OpticalBeams { header :: Header.Header
                                 , _data :: V.Vector Word.Word16
                                 , broken :: V.Vector P.Bool
                                 } deriving (P.Show, P.Eq, P.Ord, T.Typeable)

instance RosBinary OpticalBeams where
  put obj' = put (header obj') *> put (_data obj') *> put (broken obj')
  get = OpticalBeams <$> get <*> get <*> get
  putMsg = putStampedMsg

instance HasHeader OpticalBeams where
  getSequence = Header.seq . header
  getFrame = Header.frame_id . header
  getStamp = Header.stamp . header
  setSequence seq x' = x' { header = (header x') { Header.seq = seq } }

instance MsgInfo OpticalBeams where
  sourceMD5 _ = "25d94cbd426c63c2ed133231c6993f52"
  msgTypeName _ = "pr2_pretouch_sensor_optical/OpticalBeams"
acowley commented 10 years ago

This is another outstanding bug report, thank you! I can't try anything right now, but is it possible that using rostopic to send the data is causing trouble? Everything you included there looks pretty good, and the error message makes it sound like some expected field is missing.

I'll look at it tomorrow. Let me know if you make any progress on it.

rgleichman commented 10 years ago

I also tried it with the PR2 and the OpticalBeams messages being generated with this code. The roshask node behaved the same.

acowley commented 10 years ago

I can't run this code at the moment, so I'm trying to make sure the byte counts are reasonable since the relative difference of your two tests make sense (i.e. the second test is 8 bytes shorter than the first).

This {data: [100,100,100,100], broken: [true, true, false, true]} should require

data: 4 + 4_2 broken 4 + 4_1 = 20 bytes

The header is 12 bytes for the sequence number and time stamp, and then the only thing remaining is the frame ID. The frame ID is length prefixed, so that gets us to 36 bytes, and the rest of the message payload should be the ASCII bytes of the frame ID. Do you know what frame name is getting attached to the messages you're testing with?

acowley commented 10 years ago

My byte counting confirmed a suspicion. The problem was having an array of booleans. ROS serializes booleans to 8 bits, which I mimic for serialization of single Haskell Bool values. However, for a Vector of boolean values, we were falling back to the Storable instance of Bool which uses four bytes per value. I now map ROS bool values to Haskell Word8 values, and OpticalBeams messages fly freely.

rgleichman commented 10 years ago

Thank you for fixing this. However, it also allows non-boolean values to be received (and probably also sent). For instance, if running

rostopic pub -r 1 optical/right pr2_pretouch_sensor_optical/OpticalBeams '{broken: [1,2,3,4]}'

then

runhaskell SensorSubscriber.hs

prints out

I heard ya OpticalBeams {header = Header {seq = 1, stamp = (0,0), frame_id = ""}, _data = fromList [], broken = fromList [1,2,3,4]}

Even though broken is supposed to be a list of bools according to its ROS message definition.

acowley commented 10 years ago

It does, but booleans always occupy a rather awkward spot in a world that runs on C. [1,2,3,4] are all true, after all.

We might want to special case the handling of booleans in the topic serialization code, but this would involve traversing every array we send or receive. Even if every type other than Bool is transformed by the identity function, that would probably cost performance in GHC 7.6.3 (in 7.8 we could possibly use the new coerce machinery).

One option would be to bake the Bool mapping into the generated message code itself, rather than the generic serialization code. It's not much code to duplicate (V.map (> 0) and V.map fromEnum is, I think, all we'd need), and that would avoid the traversal on things that aren't Bool.