Skip to content

Conversation

@shenki
Copy link

@shenki shenki commented May 13, 2014

The Horus team found that horizontal distances in m/s were hard to get
an intuition for, particularly as the speed across land is often
used for calculating if a car can travel to the landing site in time.

This change modifies the horizontal speed to be calculated in km/h, or
m/h if the user has selected imperial units.

Signed-off-by: Joel Stanley [email protected]

@adamgreig
Copy link
Member

nice, thanks!

could we have the imperial text be "mph" rather than "m/h" please?
also it might be nicer to just keep the calculation of horizontal rate in m/s and convert it for display, as we already do for the imperial case. that way the value kept in memory remains in standard units and is just formatted for display.

@eroomde
Copy link

eroomde commented May 13, 2014

I'm not that keen on the change, as consistent units of V and H has always been useful for an intuition on glide angles at landing, to estimate how something would have come over a tree line, say, for those countries that aren't all flat desert. I also don't have a problem converting from m/s to kmh and mph in my head - to a perfectly workable (for chase car) approximation it's a multiplication by three-and-a-half and two-and-a-quarter respectively, which is pretty easy. I've also always been keen on being resistant to changing from SI as a matter of principle, but maybe I'm being The Unreasonable Man.

@shenki
Copy link
Author

shenki commented May 13, 2014

Thanks for the review Adam. I pushed another commit with your suggested changes. The m/h bit was an attempt at being consistent, but I agree that mph looks better than m/h.

eroomde, I assume you're having a dig at Australian terrain; we were chasing balloons up and down green pastured hills today, so rest assured that this change isn't motivated by flat terrain :)

I guess if you guys aren't keen on this change we can run our own instance of the mobile tracker with km/h... and Kangaroos instead of hatchbacks.

@adamgreig
Copy link
Member

Looks like you're still doing the conversion at calculation and then again at display, or am I missing something?

@shenki
Copy link
Author

shenki commented May 13, 2014

Yeah, I just noticed that. Pushed a fix.

@shenki
Copy link
Author

shenki commented May 13, 2014

I tried to test, but with no moving vehicles atm, 0 * 3600 is still 0 :)

@adamgreig
Copy link
Member

@eroomde More likely to want the speed in car-units and then estimate in your head for the cases you want glide angle, or vice versa? I agree that going from m/s to mph or kph is plenty easy mentally for a quick estimate, but I wonder which use case is more popular.

Is there any scope for displaying both? Something like Vertical Rate: -5.2m/s (11.6mph) (or kph as appropriate).

@adamgreig
Copy link
Member

@shenki sorry to nit-pick but I note the comments still say km/h, and you know what they say about out of date comments ;)

@shenki
Copy link
Author

shenki commented May 13, 2014

On Tue, May 13, 2014 at 8:56 PM, Adam Greig [email protected] wrote:

@shenki sorry to nit-pick but I note the comments still say km/h, and you know what they say about out of date comments ;)

Oh! Fixed. With bonus spell check!

@adamgreig
Copy link
Member

Doesn't look like you actually pushed the fix :p

Any thoughts on displaying both units, like mentioned above?

The Horus team found that horizontal distances in m/s were hard to get
an intuition for, particularly as the speed across land is often used
for calculating if a car can travel to the landing site in time.

This change adds a selectable option that modifies the horizontal speed
to be calculated in km/h, or m/h if the user has selected imperial
units. The default behaviour is still to use distance per second.

Signed-off-by: Joel Stanley <[email protected]>
@shenki
Copy link
Author

shenki commented May 18, 2014

@adamgreig I pushed the fix for the comment, and did some experiments with the units. I implemented displaying both distance/hour and distance/second, and it made the UI cluttered.

I also tried making it a mouse-over text, using

, except the extra div was causing a newline which made it look even worse than displaying both units.

I instead opted for a configuration option that allows switching between distance/hour and distance/second for the horizontal x-axis. I found it a hard option to name, but have a look and let me know what you think.

@rossengeorgiev
Copy link
Member

@adamgreig Is there any scope for displaying both? Something like Vertical Rate: -5.2m/s (11.6mph) (or kph as appropriate).

When I added the horizontal rate, there was the issue that if I put it as a separate parameter, I am adding 2.5 lines of text, which wastes a lot of screen space. At the end I combined vertical and horizontal rate. Their values are short enough together to fit on the same line.

@shenki The change looks ok, and I can merge it after I push my latest feature.

I am just wonder whenever, this option should be on by default. Could be more useful to people?

@rossengeorgiev
Copy link
Member

Change has been pushed to master at c5afdc1 and is live on the tracker.

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.

4 participants