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

FTP plugin #128

Closed
21 of 24 tasks
vooon opened this issue Aug 18, 2014 · 23 comments
Closed
21 of 24 tasks

FTP plugin #128

vooon opened this issue Aug 18, 2014 · 23 comments

Comments

@vooon
Copy link
Member

vooon commented Aug 18, 2014

I want to add FTP plugin when it support will stabilize.

Wait when:

Also i need reference client implementation, so wait QGC update.

Note: we start discussion in #41, but FTP is offtopic there.

Implement file operations:

  • list directory
  • open file (read)
  • read file
  • close file
  • create file (write)
  • open file (write)
  • write file
  • mkdir
  • rmdir
  • remove file
  • truncate file
  • rename
  • get crc32 for file

TODO:

  • ACK timeout
  • Return error code from open/read/write
  • Bitrate flow stats
  • Diagnostics
  • Translate protocol errors to errno

Protocol TODO:

  • open for write
  • calculate crc32 for file (might be useful for verification)
  • truncate()
  • rename file/dir
@DonLakeFlyer
Copy link

I've been bogged down by finishing up a big QGroundControl RC Calibration pull. That went in yesterday. Now I move back to FTP. First work item will be improving download speed, with that work I"ll likely switch to the new mavlink message. Implementing missing file operations will come last and may take a while to get to. Hoping for first work item complete in a week. It may take longer since my concept for streaming messages to client without Acks may or may not work.

@TSC21
Copy link
Member

TSC21 commented Aug 28, 2014

Already uses FILE_TRANSFER_PROTOCOL: PX4/PX4-Autopilot@2c72468

@vooon
Copy link
Member Author

vooon commented Sep 1, 2014

I've started to write plugin.

@DonLakeFlyer i have few questions:

  1. Why ftp adds extra crc32, while transfer layer (mavlink) already implements crc16 for datagrams?
  2. There is no commands for mkdir()/rmdir()?
  3. How i can get file stats?

vooon added a commit that referenced this issue Sep 1, 2014
@DonLakeFlyer
Copy link

  1. Hmm, Never thought about that. Just following the original implementation. I"ll ask Lorenz at some point.
    2/3. Original driver to FTP was mainly to download logs. So not much work/thought put into 2 and 3.

@DonLakeFlyer
Copy link

Talked with Lorenz. Inner CRC is not needed. I'll remove it at some point. Once I get past download streaming I'll also implement full create/delete on files and directories. Commands for that are not fully in there yet.

@vooon
Copy link
Member Author

vooon commented Sep 1, 2014

_3. I've see in FTP, size transmitted as text in dir list, so at least file size can be cached in fuse driver.

@vooon
Copy link
Member Author

vooon commented Sep 1, 2014

But i don't see reasons why dir list return absolute path, not relative as readdir() do it.

@DonLakeFlyer
Copy link

List command returned paths are relative

@vooon
Copy link
Member Author

vooon commented Sep 1, 2014

I messed up a code piece which is taken stat().

@vooon
Copy link
Member Author

vooon commented Sep 1, 2014

Also i don't like that client don't know what command return ACK/NACK (from message).

vooon added a commit that referenced this issue Sep 1, 2014
Based on QGroundContol QGCUASFileManager.h/cc.
Issue #128.
vooon added a commit that referenced this issue Sep 2, 2014
vooon added a commit that referenced this issue Sep 2, 2014
vooon added a commit that referenced this issue Sep 2, 2014
@vooon
Copy link
Member Author

vooon commented Sep 2, 2014

FTP:List works!

TODO: timeout handling.

$ rosservice call /mavros/ftp/list /fs/microsd
list: 
  - 
    name: bootlog.txt
    type: 0
    size: 543
  - 
    name: dataman
    type: 0
    size: 101504
  - 
    name: log
    type: 1
    size: 0
  - 
    name: etc
    type: 1
    size: 0
success: True

vooon added a commit that referenced this issue Sep 2, 2014
vooon added a commit that referenced this issue Sep 2, 2014
vooon added a commit that referenced this issue Sep 2, 2014
vooon added a commit that referenced this issue Sep 2, 2014
Sometimes kCmdReset can restore normal operation,
but it might be dangerous.
Issue #128.
vooon added a commit that referenced this issue Sep 2, 2014
@vooon vooon mentioned this issue Sep 2, 2014
@vooon
Copy link
Member Author

vooon commented Sep 2, 2014

Now it works (list & read)!

New script mavftp.

@TSC21
Copy link
Member

TSC21 commented Sep 2, 2014

@vooon if I want to download a file what's the procedure?

@vooon
Copy link
Member Author

vooon commented Sep 2, 2014

@TSC21 Start mavros, then use mavftp tool to list files:

rosrun mavros mavftp list /
rosrun mavros mavftp cat /etc/init.d/rc.usb > /tmp/file

Upd: FW has session leak bug, so after each 2 cat to one reset.

@vooon
Copy link
Member Author

vooon commented Sep 2, 2014

Check file size using list. But i not recommend use plugin for large files (>10K) now.
Also you can use pv to display data rate.

rosrun mavros mavftp cat /file | pv -rb -s <file size> > /tmp/file

@TSC21
Copy link
Member

TSC21 commented Sep 2, 2014

Check file size using list. But i not recommend use plugin for large files (>10K) now.

It took some time but I was able to download a 2.9MB file :) so it seems to be working. It is slow though.

Also you can use pv to display data rate.

rosrun mavros mavftp cat /file | pv -rb -s <file size> > /tmp/file

Can you include | pv -rb -s <file size> as default in the mavftp cat command? Or at least as a arg option like -r or --rate?

@vooon
Copy link
Member Author

vooon commented Sep 2, 2014

Maybe later, but mavftp goal is testing ftp plugin, not for general use.

Also cat require mavros read all file content and then return it via service, not fast on big files.

$ rosrun mavros mavftp cat /fs/microsd/log/2014-08-26/15_37_25.bin | pv -rab -s 298820 > /tmp/file.bin
 292kB [  24kB/s] [  24kB/s]

If pv is not wrong with the calculation speed, it is not that bad, with QGC like speed was 8 kB/s.

@DonLakeFlyer
Copy link

See: PX4/PX4-Autopilot#1345 for significant updates to FTP firmware. You'll have the update to the new opcodes and error codes.

@vooon
Copy link
Member Author

vooon commented Sep 10, 2014

Waiting PX4/PX4-Autopilot#1357 . Also maybe better to add file permissions on list operation.

@vooon vooon added this to the Version 0.8.0 milestone Sep 11, 2014
@vooon vooon mentioned this issue Sep 11, 2014
4 tasks
@vooon
Copy link
Member Author

vooon commented Sep 15, 2014

Found error in kCmdListDirectory.

vooon added a commit that referenced this issue Sep 15, 2014
vooon added a commit that referenced this issue Sep 15, 2014
vooon added a commit that referenced this issue Sep 15, 2014
vooon added a commit that referenced this issue Sep 16, 2014
vooon added a commit that referenced this issue Sep 16, 2014
vooon added a commit that referenced this issue Sep 17, 2014
vooon added a commit that referenced this issue Sep 17, 2014
vooon added a commit that referenced this issue Sep 17, 2014
vooon added a commit that referenced this issue Sep 17, 2014
vooon added a commit that referenced this issue Sep 17, 2014
vooon added a commit that referenced this issue Sep 22, 2014
vooon added a commit that referenced this issue Sep 22, 2014
vooon added a commit that referenced this issue Sep 22, 2014
vooon added a commit that referenced this issue Sep 22, 2014
vooon added a commit that referenced this issue Sep 22, 2014
vooon added a commit that referenced this issue Sep 22, 2014
@vooon
Copy link
Member Author

vooon commented Sep 22, 2014

I'm done. Diagnostics will be later in #154.

Now i only waits merge PX4/PX4-Autopilot#1363.

@vooon
Copy link
Member Author

vooon commented Sep 22, 2014

I think i can close issue now, so as not to slow down release.

@vooon vooon closed this as completed Sep 22, 2014
nmichael pushed a commit to rislab/mavros that referenced this issue Mar 19, 2016
Adding optional enum attribute to fields that represent an enum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants