parallaxinc / Flight-Controller

Quadcopter Flight Controller
Other
24 stars 20 forks source link

Add X-Axis Units to Sensor Graph #50

Open RoboTechie opened 7 years ago

RoboTechie commented 7 years ago

Add label to x-axis of the sensor graph in ground station with units (even if it's a somewhat useless unit like cycles)

JasonDorie commented 7 years ago

It would be useful if you could collect related things into groups. When I was previously working on the graph code, to unlink the altimeter readings and dump the output, this would've been a trivial change to make along with those. Having it as a distinct task means I have to "context switch" back in to the graph code, which takes quite a bit longer than when I'm already in there. Just an FYI.

On Wed, Oct 12, 2016 at 11:52 AM, Kyle M notifications@github.com wrote:

Add label to x-axis of the sensor graph in ground station with units (even if it's a somewhat useless unit like cycles)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/issues/50, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_qDT9aE6nRHNcJqtR3NQ8hKGXWvdks5qzSx-gaJpZM4KVGAJ .

JasonDorie commented 7 years ago

I am curious what benefit we get from having a label. I mean, if I'm just adding the word "time" or something to the bottom of the graph, isn't that painfully obvious already, and simply consuming display space?

On Wed, Oct 12, 2016 at 12:01 PM, Jason Dorie jason.dorie@gmail.com wrote:

It would be useful if you could collect related things into groups. When I was previously working on the graph code, to unlink the altimeter readings and dump the output, this would've been a trivial change to make along with those. Having it as a distinct task means I have to "context switch" back in to the graph code, which takes quite a bit longer than when I'm already in there. Just an FYI.

On Wed, Oct 12, 2016 at 11:52 AM, Kyle M notifications@github.com wrote:

Add label to x-axis of the sensor graph in ground station with units (even if it's a somewhat useless unit like cycles)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/issues/50, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_qDT9aE6nRHNcJqtR3NQ8hKGXWvdks5qzSx-gaJpZM4KVGAJ .

RoboTechie commented 7 years ago

I don't think it's that obvious, as I haven't been able to figure out what the units are.

and Regarding you previous comment Duly noted. I've been creating GitHub issues as I think of things, rather than when i have a collection of similar issues, so I don't forget them, and because I've been instructed in the past to make each issue as discrete/narrow as possible. But maybe we ought to consider organizing the issues a little differently if there way we're doing it now isn't very helpful. But to be clear, I don't expect them to be addressed immediately or in order, I'd rather leave it up to your discretion to decide what to work on when. If it's something that I think needs to be a high priority, I'll tag it with "urgent/safety", otherwise I don't mind if it waits for a while until there's a collection of issues similar to it.

JasonDorie commented 7 years ago

The sensor data isn't timestamped at all, because it would make every packet bigger. We need to keep decently below the theoretical maximum transmit rate to allow for XBee retries. I could probably include a single byte as a "sequence number", which would allow me to be sure which batch they were supposed to be in, but that's about the extent of what I'd likely do. In the firmware it works like this:

There are several reasons for breaking it up this way:

At 250 Hz, a cycle would be 250/8 hz, or roughly 30 times a second. On XBee I update half as often because the serial TX rate is halved, so it'd be roughly 15 full cycles / second at 250hz. Now that we're at 200hz, that update rate is 25 times/sec over USB, or 12.5 times/sec via XBee. The plotted data advances the current cycle every time it gets the last phase of data, so if that packet is corrupted it might not advance until the next cycle.

So the short version is, the X axis is now approximately 25ths of a second, or 12.5ths of a second, depending on USB or XBee. I might even drop the XBee update rate again - the transmission is significantly more reliable at lower baud rates, so we could drop to 38,400, and with a lower throughput rate there'd be fewer corrupted packets and probably fewer retries, which might end up reducing the lag. It would mean having to reconfigure the XBee units for the lower rate, which is a pain.

J

On Wed, Oct 12, 2016 at 1:25 PM, Kyle M notifications@github.com wrote:

I don't think it's that obvious, as I haven't been able to figure out what the units are.

and Regarding you previous comment Duly noted. I've been creating GitHub issues as I think of things, rather than when i have a collection of similar issues, so I don't forget them, and because I've been instructed in the past to make each issue as discrete/narrow as possible. But maybe we ought to consider organizing the issues a little differently if there way we're doing it now isn't very helpful. But to be clear, I don't expect them to be addressed immediately or in order, I'd rather leave it up to your discretion to decide what to work on when. If it's something that I think needs to be a high priority, I'll tag it with "urgent/safety", otherwise I don't mind if it waits for a while until there's a collection of issues similar to it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/issues/50#issuecomment-253328736, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_n4JJCVwEgQuoxlGTtF4IpPJ1Ywsks5qzUJAgaJpZM4KVGAJ .

RoboTechie commented 7 years ago

Wow. thanks for the detailed explanation. Given how complex (and of questionable net benefit) it sounds to tie the data to time, I think we should just put this whole thing on the back burner for now. I don't want to cancel it till I can talk to Matt, but ignore it in the mean time.

JasonDorie commented 7 years ago

If I added a single byte to each packet for the 'cycle', it would allow me to correlate different phases of data to a particular cycle, and detect dropped phase data too, but overall I don't think it adds much - the telemetry is largely meant as a debug / realtime view, and we can't guarantee data integrity anyway, so I haven't taken any real pains too do much more than display it.

On Wed, Oct 12, 2016 at 2:07 PM, Kyle M notifications@github.com wrote:

Wow. thanks for the detailed explanation. Given how complex (and of questionable net benefit) it sounds to tie the data to time, I think we should just put this whole thing on the back burner for now. I don't want to cancel it till I can talk to Matt, but ignore it in the mean time.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/issues/50#issuecomment-253339559, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_v5u0BbyJqK7lv0KdjPVaiYpuJ9lks5qzUwPgaJpZM4KVGAJ .

MatzElectronics commented 7 years ago

Even something approximate would be okay, but not super high priority, and we can document that it isn't exact.

Not super high priority.

JasonDorie commented 7 years ago

Kyle wasn't asking for time-stamps, but an axis label (like, literally the word "time" or "cycles" along the bottom). I haven't done this because it wastes space and is obvious. It sounds like you are now asking for timestamps on the samples?

J

On Wednesday, October 19, 2016, Matthew Matz notifications@github.com wrote:

Even something approximate would be okay, but not super high priority, and we can document that it isn't exact.

Not super high priority.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/issues/50#issuecomment-254992646, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_kDlhZCUz6DetBEYNG1ry_oSShC3ks5q1tEvgaJpZM4KVGAJ .

MatzElectronics commented 7 years ago

Sorry, just getting back into issues - I'm on the fence. Does the data export in future versions contain a cycle or timestamp column? If so, let's not worry about it on the graph, and I can close the issue.

Matthew Matz | STEM/Robotics Educator Parallax Inc. | Direct: 916-625-3019 | www.parallax.com | @M atzElectronics http://twitter.com/MatzElectronics

On Wed, Oct 19, 2016 at 10:30 PM, Jason Dorie notifications@github.com wrote:

Kyle wasn't asking for time-stamps, but an axis label (like, literally the word "time" or "cycles" along the bottom). I haven't done this because it wastes space and is obvious. It sounds like you are now asking for timestamps on the samples?

J

On Wednesday, October 19, 2016, Matthew Matz notifications@github.com wrote:

Even something approximate would be okay, but not super high priority, and we can document that it isn't exact.

Not super high priority.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/ issues/50#issuecomment-254992646, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_ kDlhZCUz6DetBEYNG1ry_oSShC3ks5q1tEvgaJpZM4KVGAJ .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/issues/50#issuecomment-254993282, or mute the thread https://github.com/notifications/unsubscribe-auth/AS0quGkqqylDlSZR7NpygSMGh7cNI5Pnks5q1tJcgaJpZM4KVGAJ .

JasonDorie commented 7 years ago

If you'd like a time stamp or cycle counter it's doable. If we're going to do it, it might as well happen with the next change, because I've already changed the packet format this release. Another change to the format in a single release is easier than changing it again in some future version.

J

On Wednesday, October 19, 2016, Matthew Matz notifications@github.com wrote:

Sorry, just getting back into issues - I'm on the fence. Does the data export in future versions contain a cycle or timestamp column? If so, let's not worry about it on the graph, and I can close the issue.

Matthew Matz | STEM/Robotics Educator Parallax Inc. | Direct: 916-625-3019 | www.parallax.com | @M atzElectronics http://twitter.com/MatzElectronics

On Wed, Oct 19, 2016 at 10:30 PM, Jason Dorie <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Kyle wasn't asking for time-stamps, but an axis label (like, literally the word "time" or "cycles" along the bottom). I haven't done this because it wastes space and is obvious. It sounds like you are now asking for timestamps on the samples?

J

On Wednesday, October 19, 2016, Matthew Matz <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Even something approximate would be okay, but not super high priority, and we can document that it isn't exact.

Not super high priority.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/ issues/50#issuecomment-254992646, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_ kDlhZCUz6DetBEYNG1ry_oSShC3ks5q1tEvgaJpZM4KVGAJ .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/ issues/50#issuecomment-254993282, or mute the thread https://github.com/notifications/unsubscribe-auth/ AS0quGkqqylDlSZR7NpygSMGh7cNI5Pnks5q1tJcgaJpZM4KVGAJ .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/issues/50#issuecomment-254993878, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_qfgJqsmPpKnlTTBFJikrnXU8HKzks5q1tN7gaJpZM4KVGAJ .

MatzElectronics commented 7 years ago

Yes - I think it's worth having output during one of the phases (not every packet). I understand that it won't be exact or that it could be slightly off if packets get lost or dropped, but otherwise there is no way to reasonably know when a measurement occurred relative to another measurement, especially if there was a period of radio loss.

JasonDorie commented 7 years ago

Looking at the code, I have the low 16 bits of the loop counter being transmitted in the cycle stats phase already, so on receipt of one of those, I can actually infer the time that packet arrived, knowing we update 200 times / sec. That counter will roll over roughly ever 5 1/2 minutes, so any outage longer than that and we won't know how long it actually was, or if you constantly drop the "phase 1" packet that contains the counter we'd have the same problem. I don't think either of those is likely.

On Wed, Oct 19, 2016 at 8:00 PM, Matthew Matz notifications@github.com wrote:

Yes - I think it's worth having output during one of the phases (not every packet). I understand that it won't be exact or that it could be slightly off if packets get lost or dropped, but otherwise there is no way to reasonably know when a measurement occurred relative to another measurement, especially if there was a period of radio loss.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/issues/50#issuecomment-254996907, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_ulUlkh-SPDuQCaMA0en46wHpn-Nks5q1tlbgaJpZM4KVGAJ .

MatzElectronics commented 7 years ago

Okay, perfect!

Matthew Matz | STEM/Robotics Educator Parallax Inc. | Direct: 916-625-3019 | www.parallax.com | @M atzElectronics http://twitter.com/MatzElectronics

On Wed, Oct 19, 2016 at 11:44 PM, Jason Dorie notifications@github.com wrote:

Looking at the code, I have the low 16 bits of the loop counter being transmitted in the cycle stats phase already, so on receipt of one of those, I can actually infer the time that packet arrived, knowing we update 200 times / sec. That counter will roll over roughly ever 5 1/2 minutes, so any outage longer than that and we won't know how long it actually was, or if you constantly drop the "phase 1" packet that contains the counter we'd have the same problem. I don't think either of those is likely.

On Wed, Oct 19, 2016 at 8:00 PM, Matthew Matz notifications@github.com wrote:

Yes - I think it's worth having output during one of the phases (not every packet). I understand that it won't be exact or that it could be slightly off if packets get lost or dropped, but otherwise there is no way to reasonably know when a measurement occurred relative to another measurement, especially if there was a period of radio loss.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/ issues/50#issuecomment-254996907, or mute the thread https://github.com/notifications/unsubscribe-auth/ANak_ulUlkh- SPDuQCaMA0en46wHpn-Nks5q1tlbgaJpZM4KVGAJ .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/Flight-Controller/issues/50#issuecomment-255001858, or mute the thread https://github.com/notifications/unsubscribe-auth/AS0quL3O0nKZDaNODWTiAt1TMCZN81Inks5q1uOggaJpZM4KVGAJ .