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

Curve item units displayed on graph #1986

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Curve item units displayed on graph #1986

wants to merge 2 commits into from

Conversation

TEAM-NAH
Copy link

It would be beneficial for the users to be able to know the units of a line as it is displayed on the Mavlink Graph log analyses page. Also fixed a typo that is displayed on the Front end application

@AppVeyorBot
Copy link

@TEAM-NAH
Copy link
Author

We would love to get feedback on this PR. If the Unit's dictionary should be placed in a resource file or if the current placement of the code is fine. Any tips on how to proceed would be appreciated

@meee1
Copy link
Contributor

meee1 commented Nov 18, 2018

I like the intent. however i feel it would be better not to have hardcoded entries in the zedgraph library
i made this change 1cb7d10
that would be more universal.
So it it where me,
modify the zedgraph library to have a unit property per line
then modify the code to pull the attribute and set the unit on the line when the data is graphed.
this makes it usable into the future. ie im thinking using something like this for dataflash logs.

@TEAM-NAH
Copy link
Author

Thanks for the great feedback. I just wanted to verify with your changes that the units are going to be displayed in some fashion on the Log Analyses Graph, is that correct? I tried pulling your commit and testing the code that you have implemented but was running into exception errors. Thanks for your fast and detailed responses! From not being able to test your code my team is not sure if we should implement our improvements further or if the changes you have committed include what we have envisioned.

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.

None yet

3 participants