Added FUSE support #55
35
fuse.go
@ -12,7 +12,7 @@ import (
|
||||
func startFuse(ctx context.Context, dev *infinitime.Device) error {
|
||||
yannickulrich marked this conversation as resolved
Outdated
|
||||
// This is where we'll mount the FS
|
||||
os.Mkdir(k.String("fuse.mountpoint"), 0755)
|
||||
yannickulrich marked this conversation as resolved
Outdated
Elara6331
commented
Use Use `os.MkdirAll` here instead so that it creates parent directories, and handle the error (just return it).
yannickulrich
commented
This is actually a bit more complicated than that. If the mountpoint already exists, fuse will crash. We also can't delete the mountpoint beforehand ( This is actually a bit more complicated than that. If the mountpoint already exists, fuse will crash. We also can't delete the mountpoint beforehand (`rm: cannot remove '/tmp/itd/mnt': Transport endpoint is not connected`). The best way to solve this should be calling the [unmount function](https://pkg.go.dev/github.com/hanwen/go-fuse/v2@v2.2.0/fuse#Server.Unmount). How would you suggest going about doing this?
Elara6331
commented
Yeah, this one is going to be a bit more complicated. The FUSE library does have a different unmount function that you could call before trying to mount the fs, but it's not exported, so we'll need to do a small hack to get access to it anyway. In the
|
||||
root := &ITNode{kind: 0}
|
||||
root := fusefs.BuildRootNode(dev)
|
||||
server, err := fs.Mount(k.String("fuse.mountpoint"), root, &fs.Options{
|
||||
MountOptions: fuse.MountOptions{
|
||||
// Set to true to see how the file system works.
|
||||
@ -32,43 +32,12 @@ func startFuse(ctx context.Context, dev *infinitime.Device) error {
|
||||
Str("target", k.String("fuse.mountpoint")).
|
||||
Send()
|
||||
|
||||
properties[0] = ITProperty{"heartrate", 2,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.WatchHeartRate(ctx)
|
||||
return converterU8(ctx, ans), err
|
||||
}}
|
||||
properties[1] = ITProperty{"battery", 3,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.WatchBatteryLevel(ctx)
|
||||
return converterU8(ctx, ans), err
|
||||
}}
|
||||
properties[2] = ITProperty{"motion", 4,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.WatchMotion(ctx)
|
||||
return converterMotionValues(ctx, ans), err
|
||||
}}
|
||||
properties[3] = ITProperty{"stepcount", 5,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.WatchStepCount(ctx)
|
||||
return converterU32(ctx, ans), err
|
||||
}}
|
||||
properties[4] = ITProperty{"version", 6,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.Version()
|
||||
return converter1String(ctx, ans), err
|
||||
}}
|
||||
properties[5] = ITProperty{"address", 7,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans := dev.Address()
|
||||
return converter1String(ctx, ans), nil
|
||||
}}
|
||||
fusefs.BuildProperties(dev)
|
||||
|
||||
myfs, err = dev.FS()
|
||||
if err != nil {
|
||||
log.Warn("Error getting BLE filesystem").Err(err).Send()
|
||||
return err
|
||||
}
|
||||
inodemap = make(map[string]uint64)
|
||||
|
||||
// Wait until unmount before exiting
|
||||
go server.Serve()
|
||||
|
@ -89,7 +89,51 @@ type ITNode struct {
|
||||
path string
|
||||
}
|
||||
|
||||
func BuildRootNode(dev *infinitime.Device) *ITNode {
|
||||
inodemap = make(map[string]uint64)
|
||||
myfs, _ = dev.FS()
|
||||
|
||||
return &ITNode{kind: 0}
|
||||
}
|
||||
|
||||
var properties = make([]ITProperty, 6)
|
||||
|
||||
yannickulrich marked this conversation as resolved
Elara6331
commented
In my opinion, "device" is unclear. I think it should be called "info" instead, that seems clearer. Another thing is that you seem to be providing continuous sensor data by never sending an EOF, which means files will be read forever. Files generally don't work like that, so it leads to consequences. Since the syscall for reading from the file will never return until new information is received, a program that reads from this file will not be able to be killed, even by In my opinion, "device" is unclear. I think it should be called "info" instead, that seems clearer.
Another thing is that you seem to be providing continuous sensor data by never sending an EOF, which means files will be read forever. Files generally don't work like that, so it leads to consequences. Since the syscall for reading from the file will never return until new information is received, a program that reads from this file will not be able to be killed, even by `SIGKILL`. You can see this if you try to `cat` the file. It can also break your system's shell depending on how it's configured. Instead, I think you should just provide single data points, and people will just have to use `itctl` if they want continuous data, because files aren't meant to work this way.
yannickulrich
commented
Happy to change that. Regarding the continuous data stream, isn't this how device files work under Linux? If I run I'm not 100% sure what you mean with the
In the Happy to change that.
Regarding the continuous data stream, isn't this how device files work under Linux? If I run `cat /dev/hidraw0` I get a live stream of data. I was hoping to get the same behaviour. I'm happy to try and fix any bug but think this is probably the best way of doing this. Alternatively, we can have to files, a live stream file (eg. `/tmp/itd/mnt/live/motion`) and a one-shot file (eg. `/tmp/itd/mnt/info/motion`). Would you be opposed to this?
I'm not 100% sure what you mean with the `cat` example. This works perfectly for me:
```
$ cat /tmp/itd/mnt/info/motion
22 -5 -1042
22 -5 -1042
22 -5 -1042
19 -5 -1041
20 -5 -1042
18 -6 -1038
19 0 -1041
24 0 -1043
23 -6 -1041
^C
$
```
In the `itd` log I also see the [Done method of the watcher](https://gitea.arsenm.dev/Arsen6331/infinitime/src/branch/master/infinitime.go#L693-L697) getting called. Could you perhaps elaborate on the problem you see?
Elara6331
commented
Try this with something that returns data less commonly, such as Linux does this kind of stuff with a char device file. Unfortunately, FUSE doesn't provide a way to make a char device. There seems to be a workaround, but it only works for the root user, so it would not work in ITD. I did test that, and I can confirm it doesn't work. > Could you perhaps elaborate on the problem you see?
Try this with something that returns data less commonly, such as `battery`. Every time `cat` tries to read from the file, it executes a syscall. Since data isn't returned until ITD receives it from the watch, the syscall doesn't return until then either. What this means is that whatever program is reading the file will be executing kernel code, which prevents you from killing it.
Linux does this kind of stuff with a char device file. Unfortunately, FUSE doesn't provide a way to make a char device. There seems to be a workaround, but it only works for the root user, so it would not work in ITD. I did test that, and I can confirm it doesn't work.
yannickulrich
commented
Okay, fair enough. Sorry for doubting you. How do you feel about keeping a stream-type file for motion? I'm not 100% sure how much sense the instantaneous motion makes.. Otherwise, I'm going to remove the entire channel stuff. Okay, fair enough. Sorry for doubting you. How do you feel about keeping a stream-type file for motion? I'm not 100% sure how much sense the instantaneous motion makes.. Otherwise, I'm going to remove the entire channel stuff.
Elara6331
commented
Yeah, instantaneous motion data doesn't make much sense, but the continuous motion data file also doesn't work all the time. InfiniTime will stop sending motion data if you have Raise To Wake and ShakeWake disabled and you put the watch to sleep with the button. This will cause programs to once again read forever without the ability to be killed, so I think it would be better to remove the continuous data entirely. I would love to have it, but if it breaks most programs, it's not worth it, especially since Yeah, instantaneous motion data doesn't make much sense, but the continuous motion data file also doesn't work all the time. InfiniTime will stop sending motion data if you have Raise To Wake and ShakeWake disabled and you put the watch to sleep with the button. This will cause programs to once again read forever without the ability to be killed, so I think it would be better to remove the continuous data entirely. I would love to have it, but if it breaks most programs, it's not worth it, especially since `itctl` can be used instead.
|
||||
func BuildProperties(dev *infinitime.Device) {
|
||||
properties[0] = ITProperty{"heartrate", 2,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.WatchHeartRate(ctx)
|
||||
return converterU8(ctx, ans), err
|
||||
}}
|
||||
properties[1] = ITProperty{"battery", 3,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.WatchBatteryLevel(ctx)
|
||||
return converterU8(ctx, ans), err
|
||||
}}
|
||||
properties[2] = ITProperty{"motion", 4,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.WatchMotion(ctx)
|
||||
return converterMotionValues(ctx, ans), err
|
||||
}}
|
||||
properties[3] = ITProperty{"stepcount", 5,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.WatchStepCount(ctx)
|
||||
return converterU32(ctx, ans), err
|
||||
}}
|
||||
properties[4] = ITProperty{"version", 6,
|
||||
yannickulrich marked this conversation as resolved
Outdated
Elara6331
commented
You should definitely handle this error You should definitely handle this error
yannickulrich
commented
Done in Done in a54ca7a
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans, err := dev.Version()
|
||||
return converter1String(ctx, ans), err
|
||||
}}
|
||||
properties[5] = ITProperty{"address", 7,
|
||||
func(ctx context.Context) (<-chan []byte, error) {
|
||||
ans := dev.Address()
|
||||
Elara6331 marked this conversation as resolved
Outdated
yannickulrich
commented
How would you recommend doing this? I suppose it could fail for all sorts of reasons such as
In other places such as when we open a file, we could have
Again, I'm sorry but I'm not a Go expert and don't really now how to do this properly.. especially when dealing with How would you recommend doing this? I suppose it could fail for all sorts of reasons such as
* no such file or directory `ENOENT`
* generic input/output error `EIO`
* invalid argument `EINVAL`
* connection aborted `ECONNABORTED`
* connection refused `ECONNREFUSED`
* connection reset `ECONNRESET`
* is actually a file `EISNAM`
In other places such as when we open a file, we could have
* is actually a folder `EISDIR`
* file exists `EEXIST`
Again, I'm sorry but I'm not a Go expert and don't really now how to do this properly.. especially when dealing with `FSError`
Elara6331
commented
The In this case, I'd use a Go type switch to check which error type actually occurred and then check the code or do whatever else needs to be done. Maybe there could be a function like If you don't feel comfortable doing that, I can merge this and then implement it myself and send you the commit so you can see how I did it, or you can just try it yourself, whatever you feel would be better. The `err` can be several different kinds of errors, and `FSError` is just one of them. It's actually a type I made. You can see it here: https://gitea.arsenm.dev/Arsen6331/infinitime/src/commit/512d48bc2469/blefs/error.go#L20. It contains an error code you can check to see what went wrong, and you can scroll down to see the meaning of each code.
In this case, I'd use a Go type switch to check which error type actually occurred and then check the code or do whatever else needs to be done. Maybe there could be a function like `syscallErr()` that takes an `error` and returns the proper syscall error?
If you don't feel comfortable doing that, I can merge this and then implement it myself and send you the commit so you can see how I did it, or you can just try it yourself, whatever you feel would be better.
yannickulrich
commented
Something like in 4c59561a99? There are a few Something like in 4c59561a99? There are a few `TODO` where I'm not sure what the correct POSIX error would be and improvised. If you have a better idea, feel free to change them though
Elara6331
commented
That looks good, thanks That looks good, thanks
|
||||
return converter1String(ctx, ans), nil
|
||||
}}
|
||||
|
||||
}
|
||||
|
||||
|
||||
|
||||
var myfs *blefs.FS = nil;
|
||||
var inodemap map[string]uint64 = nil;
|
||||
|
||||
|
This function should be called
startFUSE
instead to adhere to Go naming conventionsThanks, I'm relatively new to Go, sorry for these issues
Done in
673383f