Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

FTP coding status #1324

Closed
TSC21 opened this issue Aug 26, 2014 · 14 comments
Closed

FTP coding status #1324

TSC21 opened this issue Aug 26, 2014 · 14 comments

Comments

@TSC21
Copy link
Member

TSC21 commented Aug 26, 2014

Hi,

I would like to get the answers to the topics in this mavros issue, regarding the current status of the FTP implementation.

@LorenzMeier and @DonLakeFlyer can you please set up the current status of FTP based on the topics on mavros issue? Thanks in advance!

@DonLakeFlyer
Copy link
Contributor

Answered on macros Issue

On Aug 26, 2014, at 10:18 AM, TSC21 notifications@github.com wrote:

Hi,

I would like to get the answers to the topics in this mavros issue, regarding the current status of the FTP implementation.

@LorenzMeier and @DonLakeFlyer can you please set up the current status of FTP based on the topics on mavros issue? Thanks in advance!


Reply to this email directly or view it on GitHub.

@TSC21
Copy link
Member Author

TSC21 commented Aug 26, 2014

Thanks for the answer @DonLakeFlyer 👍 We will be waiting for those improvements :)

@vooon
Copy link
Contributor

vooon commented Sep 1, 2014

I think extra crc32 calculation not needed since it use FILE_TRANSFER_PROTOCOL.

Also i have some questions: mavlink/mavros#128

@vooon
Copy link
Contributor

vooon commented Sep 2, 2014

I found the sessions leak. kCmdTerminate don't free session.

After: https://github.com/PX4/Firmware/blob/master/src/modules/mavlink/mavlink_ftp.cpp#L434
Should be _session_fds[hdr->session] = -1;.

@DonLakeFlyer
Copy link
Contributor

Yes I have that fixed locally, plus unit test to test it all. Hope to push tonight.

On Sep 2, 2014, at 9:31 AM, Vladimir Ermakov notifications@github.com wrote:

I found the sessions leak. kCmdTerminate don't free session.

After: https://github.com/PX4/Firmware/blob/master/src/modules/mavlink/mavlink_ftp.cpp#L434
Should be _session_fds[hdr->session] = -1;.


Reply to this email directly or view it on GitHub.

@vooon
Copy link
Contributor

vooon commented Sep 15, 2014

I fond new error in server response. It sends duplicates in kCmdListDirectory on /etc:

2014-09-15 12:09:34

@vooon
Copy link
Contributor

vooon commented Sep 15, 2014

And the bug source is dir offset calculation (we skip "." & "..").

[DEBUG] [1410783884.895702201]: FTP:m: kCmdListDirectory: /etc off: 0
[DEBUG] [1410783884.895846751]: FTP:sm: SEQ(2) SESS(0) OPCODE(3) SZ(4) OFF(0)
[DEBUG] [1410783884.904178280]: FTP:rm: SEQ(3) SESS(0) OPCODE(128) RQOP(3) SZ(33) OFF(0)
[DEBUG] [1410783884.904266852]: FTP:m: ACK List SZ(33) OFF(0)
[DEBUG] [1410783884.904310005]: FTP:List Dir: mixers
[DEBUG] [1410783884.904352505]: FTP:List Dir: init.d
[DEBUG] [1410783884.904394944]: FTP:List Dir: logging
[DEBUG] [1410783884.904436809]: FTP:List Dir: extras
[DEBUG] [1410783884.904481230]: FTP:m: kCmdListDirectory: /etc off: 4
[DEBUG] [1410783884.904519652]: FTP:sm: SEQ(3) SESS(0) OPCODE(3) SZ(4) OFF(4)
[DEBUG] [1410783884.915044309]: FTP:rm: SEQ(4) SESS(0) OPCODE(128) RQOP(3) SZ(8) OFF(4)
[DEBUG] [1410783884.915146381]: FTP:m: ACK List SZ(8) OFF(4)
[DEBUG] [1410783884.915198973]: FTP:List Dir: extras
[DEBUG] [1410783884.915275383]: FTP:m: kCmdListDirectory: /etc off: 5
[DEBUG] [1410783884.915322233]: FTP:sm: SEQ(4) SESS(0) OPCODE(3) SZ(4) OFF(5)
[DEBUG] [1410783884.923033359]: FTP:rm: SEQ(5) SESS(0) OPCODE(129) RQOP(3) SZ(1) OFF(5)
[DEBUG] [1410783884.923111918]: FTP:List done

@vooon
Copy link
Contributor

vooon commented Sep 15, 2014

Also skipping unknown entries (prev 'U') breaks too. I think we may add 'S' dirent type (skipped).
It requires less space than old 'U', just 2 bytes: 'S', '\0'.

@DonLakeFlyer
Copy link
Contributor

Ahh, I see the problem. Thanks. Just need to bump offset even though the results aren’t sent back,

On Sep 15, 2014, at 5:40 AM, Vladimir Ermakov notifications@github.com wrote:

Also skipping unknown entries (prev 'U') breaks too. I think we may add 'S' dirent type (skipped).
It requires less space than old 'U', just 2 bytes: 'S', '\0'.


Reply to this email directly or view it on GitHub.

@vooon
Copy link
Contributor

vooon commented Sep 15, 2014

@DonLakeFlyer please look my PR's.

@DonLakeFlyer
Copy link
Contributor

Will do. Probably not till tonight. Also, my comment below is incorrect as I’m sure you already know.

On Sep 15, 2014, at 10:26 AM, Donald Gagne don@thegagnes.com wrote:

Ahh, I see the problem. Thanks. Just need to bump offset even though the results aren’t sent back,

On Sep 15, 2014, at 5:40 AM, Vladimir Ermakov notifications@github.com wrote:

Also skipping unknown entries (prev 'U') breaks too. I think we may add 'S' dirent type (skipped).
It requires less space than old 'U', just 2 bytes: 'S', '\0'.


Reply to this email directly or view it on GitHub.

@LorenzMeier
Copy link
Member

@DonLakeFlyer I presume this is closed?

@vooon
Copy link
Contributor

vooon commented Oct 8, 2014

Yes.

@DonLakeFlyer
Copy link
Contributor

Yup

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants