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

NTPd Autoset GPS device baud rate. Issue #7284 #4213

Merged
merged 1 commit into from Mar 11, 2020

Conversation

vktg
Copy link
Contributor

@vktg vktg commented Feb 29, 2020

It would be nice to have the option to attempt to auto configure the baud rate for a GPS device.
Useful if you don't know the baud rate of your device.
Or for some devices that lose their configurations during power outages.
Or when changing the baud rate.

This is resolved copy of the original PR #3561 by @jskyboo

@vktg vktg force-pushed the ntpgpsautoset branch 3 times, most recently from 041c48f to 3185148 Compare February 29, 2020 06:57
usleep(1000);
// Set timeout to 5s
$timeout=microtime(true)+5;
if($fp = fopen($device, 'r')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a space between if and (

$c = fread($fp, $buffersize - $cnt);

// Wait for data to arive
if($c === false || strlen($c) == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a space between if and (.
Each test in the if needs its own set of parenthesis.


$contents.=$c;
$cnt = $cnt + strlen($c);
} while($cnt < $buffersize && microtime(true)<$timeout);
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a space between while and (.
Needs spaces around < in the second test.
Each test needs its own set of parenthesis.


$nmeasentences = ['RMC', 'GGA', 'GLL', 'ZDA', 'ZDG', 'PGRMF'];
foreach ($nmeasentences as $sentence) {
if(strpos($contents, $sentence) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a space between if and (

if (strpos($contents, '0') > 0) {
$filters = ['`', '?', '/', '~'];
foreach ($filters as $filter) {
if(strpos($contents, $filter) !== false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a space between if and (

break;
}
}
if($found === false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a space between if and (.

/* XXX: Why not file_put_contents to the device */
@file_put_contents('/tmp/gps.init', $gps_init);
mwexec("cat /tmp/gps.init > {$serialport}");
@file_put_contents($gps_device, $gps_init);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does that actually work as expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unable to check now (no GPS device available)
I can leave this strings as-is, or test it later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll test @file_put_contents ($ gps_device, $ gps_init); when I get the gps device
Now I leave it as-is

@file_put_contents($gps_device, $gps_init);

if ($found && $config['ntpd']['gps']['autobaudinit']) {
mwexec("stty -f {$serialport} raw speed {$gpsbaud}");
Copy link
Contributor

Choose a reason for hiding this comment

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

See previous comments about stty and variables in the command.

/* Remove old /etc/remote entry if it exists */
if (mwexec("/usr/bin/grep -c '^gps0' /etc/remote")) {
mwexec("/usr/bin/sed -i '' -n '/gps0/!p' /etc/remote");
}

/* Add /etc/remote entry in case we need to read from the GPS with tip */
if (intval(`grep -c '^gps0' /etc/remote`) == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix this grep to use the full path while you're at it

@@ -198,9 +198,18 @@ function autocorrect_initcmd($initcmd) {

if (!empty($_POST['gpsspeed'])) {
$config['ntpd']['gps']['speed'] = $_POST['gpsspeed'];
if($_POST['gpsspeed'] == 'autoalways') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a space between if and (.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all fixed

$c = fread($fp, $buffersize - $cnt);

// Wait for data to arive
if ($c === false || strlen($c) == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Each test requires its own set of parenthesis

64 => '57600',
80 => '115200'
);
$revspeeds = array_flip($speeds);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do an array_flip() here? You only use this variable once below. It would be better to skip this and use array_search() later.

Comment on lines 1622 to 1623
mwexec("/bin/stty -f " . escapeshellarg($serialport) . " raw speed " . escapeshellarg($gpsbaud));
mwexec("/bin/stty -f " . escapeshellarg($serialport) . ".init raw speed " . escapeshellarg($gpsbaud));
Copy link
Contributor

Choose a reason for hiding this comment

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

Since these two lines are practically the same here and when used again, it would probably be beneficial to make a function that takes the port and baud as arguments, and then call that from both places. That way if the init process changes in the future, only one place needs to be updated.

Comment on lines 1629 to 1630
mwexec("/bin/stty -f " . escapeshellarg($serialport) . " raw speed " . escapeshellarg($baud));
mwexec("/bin/stty -f " . escapeshellarg($serialport) . ".init raw speed " . escapeshellarg($baud));
Copy link
Contributor

Choose a reason for hiding this comment

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

See previous note about making this a function

Comment on lines 1659 to 1660
mwexec("/bin/stty -f " . escapeshellarg($serialport) . " raw speed " . escapeshellarg($gpsbaud));
mwexec("/bin/stty -f " . escapeshellarg($serialport) . ".init raw speed " . escapeshellarg($gpsbaud));
Copy link
Contributor

Choose a reason for hiding this comment

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

See previous note about making this a function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all fixed

@netgate-git-updates netgate-git-updates merged commit ee50de4 into pfsense:master Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants