-
Notifications
You must be signed in to change notification settings - Fork 19
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
Faraday Software - Simple Scripts #246
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved but please make the simple changes I suggested.
simple_scripts/faraday_program.py
Outdated
|
||
# Proxy | ||
proxy_callsign = 'REPLACEME' | ||
proxy_nodeid = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard configured nodeid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, TravisCI fails otherwise. Adding a comment "Replace with proxy_nodeid of local unit to be programmed"
call(['faraday-deviceconfiguration', '--init-faraday-config']) # Initialize faraday configuration INI holding program settings | ||
|
||
# Basics | ||
call(['faraday-deviceconfiguration', '--proxycallsign', proxy_callsign, '--proxynodeid', str(proxy_nodeid), '--nodeid', str(proxy_nodeid), '--callsign', proxy_callsign]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all these faraday-deviceconfiguration
calls run at once or discretely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discretely. Minimal efficiency drop and easier to read. I realize now we could optimize by just building a concatenated string and pulling one (or minimal needed in proxy case) call() functions. I think we can pass on this for the PR given low risk.
simple_scripts/faraday_program.py
Outdated
|
||
# Start servers | ||
print ("--- STARTING PROXY SERVER ---") | ||
print type(running_os), running_os == 'linux2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must this change if you're running windows? Seems like it. At least comment that you need to change this. This also just looks like a comparison...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was old debug print I missed. Removed. line 63.
simple_scripts/faraday_start.py
Outdated
# Proxy | ||
proxy_unitcnt = 1 # Number of units proxy is connecting to (starts at unit0) | ||
proxy_unit0_callsign = 'REPLACEME' | ||
proxy_unit0_nodeid = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard coded nodeid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep just like your prior comment. Updating with a comment "Replace with proxy_nodeid of local unit to be programmed"
print ("--- INITIALIZING PROXY SERVER ---") | ||
call(['faraday-proxy', '--init-config']) | ||
call(['faraday-proxy', '--unit', str(0), '--callsign', proxy_unit0_callsign, '--nodeid', str(proxy_unit0_nodeid), '--port', proxy_unit0_port]) | ||
#call(['faraday-proxy', '--unit', str(1), '--callsign', proxy_unit1_callsign, '--nodeid', str(proxy_unit1_nodeid), '--port', proxy_unit1_port]) # Uncomment if second unit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be mentioning that these commented out lines are for a second radio.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment added.
@@ -0,0 +1,88 @@ | |||
# Faraday - Simple Scripts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that these are largely development scripts and not meant for mainstream use at this time. Basically they are only available if you checkout git/download them manually and we assume you know what you're doing if you do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted.
default_altitude='00000.00' | ||
``` | ||
|
||
### GPS Defaults `DDMM.mmmm` format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an awkward section. Also, longitude is DDDMM.mmmm
while latitude is DDMM.mmmm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, keeping as is for wording right now. Fixed notation.
simple_scripts/readme.md
Outdated
DDMM.mmmm Lon (RAW) = `W 118 26.171` | ||
``` | ||
|
||
Converting into a valid Faraday configuration: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, added link and some text about getting coordinates from google maps. Conversion is the same. Pushing forwarding. Will update further if needed (getting questions from users).
Yes this works on Windows 7 as a development script. Definitely a bit rough around the edges but it's incredibly useful for dev work. I'd like to see it become some sort of program in the future. Likely just One thing to note about GPS coordinates. I tried to convert: 34.055975, -118.246283 Into DDMM format and was told: N 34 3.358, W 118 14.777 In reality this converts to: default_latitude = '3403.3580' I know you mentioned it in the readme but it's actually still not very clear that the latitude is '34' + '03'. That website should be giving the leading '0' for the '03'... |
This Pull Request adds basic scripts to:
from a single cross-platform (Windows/Linux) script respectively. These scripts are to be simplistic and will likely be replaced by a user-interface command program but until then these scripts provide a lot of functionality.
I've asked both @kb1lqc and @reillyeon to review but only one is really needed. I'd like someone else to at least run this is both Windows and Linux. Also, Should this be kicked back to adding support for Mac's as well?
Issue References