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

Livetime interface updates #78

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

Conversation

ErikBorra
Copy link

This pull request includes the timingserver layout from @basdelfos (up til #67f5736), including:

  • a nicer layout
  • pi temperature
  • shutdown and restart button for pi

I have further tweaked it by

  • using the full width of the window instead of Bas' default 900px
  • adding a clearfix for lap times so that nodes 4 to 8 stay on the same height
  • adding all fpv frequencies as a drop down

@punkkills
Copy link
Collaborator

Hey @ErikBorra, I'll give this change a more in depth view when i have some time. Out of curiosity, what value do you find that these changes add? When I built this page, the intent was a place to test out the Socket.IO API as I was working with Cory for the Livetime integration. I didn't really intend this page to be a long lived user facing UI. All of these values are easily accessible from LiveTime. If the value is the shutdown and temperature reading, those are things better requested from LiveTime, IMO.

@ErikBorra
Copy link
Author

Hi @punkkills,

thanks for your feedback. There are a few uses for the page:

  • shutdown/restart buttons
  • easier monitoring of correct calibration settings
  • monitoring the logs
  • rssi graphs display (livetime doesn't show rssi values)
  • better looking interface for future led control integration

@scottgchin
Copy link
Owner

I agree with @punkkills that this page shouldn't be used to change the timer settings, as these are available through Livetime. But I think there are items that are helpful like the graphs. I was planning on officially adding those at some point (it was already there, just not linked). I also want to add the LED controls since LiveTime has limited commands.

What if we do both? Remove the timer settings, and reconfigure the page to show the graphs and the LED controls?

@ErikBorra
Copy link
Author

Why should the timer settings not be changed through that page? It doesn't hurt, does it? Actually, I find it easier to change the settings via the page, as you get the direct visual feedback of the three coloured bars (rssi peak, calibration offset, current rssi) below.

@basdelfos
Copy link

I have an updated version of my original version of this page which also displays the looptime per node and also displays the input voltage (handy when using a lipo to power the Delta 5 or when using PoE adapter with very long UTP cable). The input voltage requires a hardware mod with voltage divider and an updated sketch on node 4.

I can upload the changes to github when interested.

Copy link
Collaborator

@punkkills punkkills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have time right now, but i'd like to pull this to my machine and give it a run. My two main comments:

  1. This should still work on windows for testing purposes. The platform specific stuff seems like it will cause problems with that.

  2. In general, I would prefer to make specific requests for livetime functionality over enhancing (and maintaining) the timing server web page. You mention it is easier to make changes through the page. Let's identify the issues with the livetime integration and request to make it better. I'm sure we can ask Cory for the shutdown, temperature and voltage UIs.

@@ -10,6 +12,12 @@ def __init__(self):
self.start_time = datetime.now()
self.filter_ratio = 50

def get_cpu_temperature(self):
"""get cpu temperature using vcgencmd"""
process = Popen(['vcgencmd', 'measure_temp'], stdout=PIPE)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BaseHardwareInterface is designed to be a platform agnostic interface layer. MockInterface is a subclass and is used for testing on windows. I assume this is pi specific?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is pi (linux) specific. Should we make a PiInterface to move it out of BaseHardwareInterface?

@@ -25,6 +26,63 @@

hardwareInterface = get_hardware_interface()

# frequencies
band_channel_frequency = []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just embed the frequencies in the index.html?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes a lot of sense. I added it to server.py, as the delta5server does something similar at L952

@ErikBorra
Copy link
Author

ErikBorra commented May 2, 2018

Hi @punkkills,

thanks so much for your time and feedback. I'm sure many would love to have the LiveTime integration be better.

From the top of my head, the following would be great to have in LiveTime:

  • shutdown / restart
  • temperature
  • voltage reading (as @basdelfos has implemented)
  • indicators for current rssi, peak rssi and calibration offset
  • a button to 'copy event frequencies' like with the 8-way laprf

Currently LiveTime only sends a message to delta 5 when a race has started (by calling reset_calibration in server.py). For integration with the led functionality, It'd be awesome if LiveTime could also indicate to delta5:

  • when there was a false start or end of race
  • the colours associated to a specific frequency

Last, but not least, if @lephiloux 's per node tuning was added to @scottgchin's repo (see this commit]), it'd be great if we could do that via LiveTime as well.

Shall I open an issue to discuss this, instead of discussing this here?

@basdelfos
Copy link

I've just merged the voltage and looptime monitoring to master of my fork.
https://github.com/basdelfos/delta5_race_timer

@lephiloux
Copy link
Contributor

lephiloux commented May 2, 2018 via email

os.system("sudo shutdown now")

@socketio.on('restart_pi')
def on_shutdown_pi():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must this be on_restart_pi() ?

@@ -7,6 +7,8 @@
<script src="./static/chartjs/Chart.bundle.js"></script>
<script type="text/javascript" src="./static/jquery/jquery-3.1.1.min.js"></script>
<script type="text/javascript" src="./static/socket.io/1.3.5/socket.io.min.js"></script>
<link rel="stylesheet" href="./static/bootstrap-3.3.7/css/bootstrap.min.css"></link>
<script type="text/javascript" src=".//static/bootstrap-3.3.7/js/bootstrap.min.js"></script>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<script type="text/javascript" src=".//static/bootstrap-3.3.7/js/bootstrap.min.js"></script>
<script type="text/javascript" src="./static/bootstrap-3.3.7/js/bootstrap.min.js"></script>

One / too much?

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.

6 participants