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

ffac-update-location-gps: initial package version #128

Merged
merged 6 commits into from
Aug 29, 2024

Conversation

maurerle
Copy link
Member

@maurerle maurerle commented Jul 25, 2024

This package configures gluon to update the location based on tty output of an attached GPS device (for example Navilock).

When a USB device which provides a tty is attached, it updates the location based on the output of it.
This is done using a lua rewrite of the script from the original forum posting.
It seems to work without coreutils-stty installed, which did fail the installation when selected as package dependency.

The location is only updated in memory - and only if a valid GPS fix is available.
After a reboot, the old location from the config is set.

hotplug limitation

After a sysupgrade/autoupdate - the USB dongle has to be unplugged and plugged in once for the detection to work again.
For reboots, this works fine as is.

@maurerle maurerle changed the title ffac-update-location-gp: initial package version ffac-update-location-gps: initial package version Jul 25, 2024
@maurerle maurerle force-pushed the update-location-gps branch 2 times, most recently from 78461db to c49134d Compare July 26, 2024 11:04
[ "${ENABLED}" != "1" ] && exit 0
test -e "${LOCKPATH}" && exit 0
if test -e "${TTYPATH}"; then
TTY_NAME=$(find "${TTYPATH}" -mindepth 1 -maxdepth 1 -type d 2>/dev/null | sed -n 's|.*/tty/||p')
Copy link
Contributor

@grische grische Aug 2, 2024

Choose a reason for hiding this comment

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

On my machine, this seems to yield several TTY devices and not a single device. Is this expected? Or did I use the wrong path?

# find /sys/devices/virtual/tty -mindepth 1 -maxdepth 1 -type d | sed -n 's|.*/tty/||p' | wc -l
515

A ls ${TTYPATH}* would probably be easier and doesn't require sed. Or if you want the full path, a literal ${TTYPATH}* (or an echo ${TTYPATH}*) will be fine.

As this is a local variable, I would also make it lowercase instead, so shellcheck can check its usage across the file.

Copy link
Member Author

@maurerle maurerle Aug 7, 2024

Choose a reason for hiding this comment

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

The USB path in DEVPATH is something like /devices/platform/1f400000.fpi/1e101000.usb/usb1/1-1/1-1:1.0 so there exists /sys/devices/platform/1f400000.fpi/1e101000.usb/usb1/1-1/1-1:1.0/tty/ttyACM0.
From this, one can use cat /dev/ttyACM0 to access the output of the USB GPS device.
So I am using the last part of the DEVPATH - this does only yield one device per USB.

Though there exists the TTY section for hotplugs, which seems more fitting here:
https://openwrt.org/docs/guide-user/base-system/hotplug#tty

So I will switch to this..

Edit: if I switch to TTY then I also see the other TTYs which I want to ignore - if a different USB TTY is attached the current version would still create the lockfile and does not react to other attached GPS devices anymore :D looks harder than expected to move away from the current check

Though it would be great to support internal TTY devices as well as there seem to be devices which do have GPS internally available like the GL-X3000..
So we need a way to detect if a connected TTY is a GPS device..

Copy link
Member Author

Choose a reason for hiding this comment

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

Now we have a lock file per TTY which waits.
This results in one process per TTY which read the other terminals to see if they also have GPS output - as I do not have a better way to detect if the TTY has this capability.

I don't think that this is a very good way to deploy this package for all models, but it seems the least harmful way which supports all cases of GPS devices.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that this is a very good way to deploy this package for all models

How would you change it?

@maurerle maurerle force-pushed the update-location-gps branch from f3ea2fe to 1f0018d Compare August 8, 2024 12:48
@maurerle maurerle requested a review from grische August 8, 2024 13:07
@maurerle maurerle force-pushed the update-location-gps branch from 1f0018d to 61af535 Compare August 23, 2024 15:37
@maurerle maurerle merged commit 102e43e into freifunk-gluon:master Aug 29, 2024
4 checks passed
@maurerle maurerle deleted the update-location-gps branch August 29, 2024 07:38
[ "${ENABLED}" != "1" ] && exit 0
test -e "${LOCKPATH}" && exit 0
if test -e "${TTYPATH}"; then
TTY_NAME=$(find "${TTYPATH}" -mindepth 1 -maxdepth 1 -type d 2>/dev/null | sed -n 's|.*/tty/||p')
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that this is a very good way to deploy this package for all models

How would you change it?

Copy link
Contributor

@grische grische left a comment

Choose a reason for hiding this comment

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

Only some comments on logging left


local lockfd, err = fcntl.open(lockfilename, bit.bor(fcntl.O_WRONLY, fcntl.O_CREAT), 384) -- mode 0600
if not lockfd then
print('err', err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure about this print format here. Above you are using print("Error: Unable to open " .. tty_device) and below print("GPS position is Invalid...", valid).

Some kind of uniform error or success printing would be nice to make it easier debug issues where this is deployed.

@maurerle
Copy link
Member Author

maurerle commented Sep 2, 2024

I found out that

root@ffac-hg-6431-ez:~# dmesg | grep TTY
[   49.544444] hotplug-update-location-gps: TTY device console was plugged in
[   50.739286] hotplug-update-location-gps: TTY device ttyLTQ0 was plugged in
[   66.107845] hotplug-update-location-gps: TTY device ttyACM0 was plugged in

shows the 3 terminals attached - so if USB is inserted, all three terminals receive a hotplug micron.d with an open terminal

But if no USB is attached, nothing shows up in dmesg | grep TTY and not TTY is registered. So it seems that this does not acquire resources on devices with nothing attached.
So I think that this is fine - it runs successfully without problems in FFAC Firmware v2023.2.3-6 :)

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.

2 participants