Added Navigation service #5

Merged
Arsen6331 merged 9 commits from yannickulrich/infinitime:navigation-service into master 3 months ago

InfiniTime implements a Navigation Service. This pull request will add it to the go library by defining a function

func (i *Device) Navigation(flag string, narrative string, dist string, progress uint8) error {
  ...
}

From the InfiniTime manual

  • flag: the graphic instruction as provided by Pure Maps. A list of valid instruction icons can be found here
  • narrative: the instruction in words, eg. "At the roundabout take the first exit".
  • dist: a short string describing the distance to the upcoming instruction such as "50 m".
  • progress: the percent complete in a uint8

Adding this to the itd daemon is straightforward

diff --git a/api/types.go b/api/types.go
index 281a85b..14c84de 100644
--- a/api/types.go
+++ b/api/types.go
@@ -22,6 +22,13 @@ type FwUpgradeData struct {
        Files []string
 }

+type NavigationData struct {
+       Flag         string
+       Narrative    string
+       Dist         string
+       Progress     uint8
+}
+
 type NotifyData struct {
        Title string
        Body  string
diff --git a/socket.go b/socket.go
index 6fcba5c..91b37c0 100644
--- a/socket.go
+++ b/socket.go
@@ -204,6 +204,10 @@ func (i *ITD) Address(_ *server.Context) string {
        return i.dev.Address()
 }

+func (i *ITD) Navigation(_ *server.Context, data api.NavigationData) error {
+       return i.dev.Navigation(data.Flag, data.Narrative, data.Dist, data.Progress)
+}
+
 func (i *ITD) Notify(_ *server.Context, data api.NotifyData) error {
        return i.dev.Notify(data.Title, data.Body)
 }
InfiniTime implements a [Navigation Service](https://github.com/InfiniTimeOrg/InfiniTime/blob/develop/doc/NavigationService.md). This pull request will add it to the go library by defining a function ```go func (i *Device) Navigation(flag string, narrative string, dist string, progress uint8) error { ... } ``` From the InfiniTime manual * `flag`: the graphic instruction as provided by [Pure Maps](https://github.com/rinigus/pure-maps/tree/master/qml/icons/navigation). A list of valid instruction icons can be found [here](https://github.com/rinigus/pure-maps/tree/master/qml/icons/navigation) * `narrative`: the instruction in words, eg. "At the roundabout take the first exit". * `dist`: a short string describing the distance to the upcoming instruction such as "50 m". * `progress`: the percent complete in a `uint8` Adding this to the `itd` daemon is straightforward ```patch diff --git a/api/types.go b/api/types.go index 281a85b..14c84de 100644 --- a/api/types.go +++ b/api/types.go @@ -22,6 +22,13 @@ type FwUpgradeData struct { Files []string } +type NavigationData struct { + Flag string + Narrative string + Dist string + Progress uint8 +} + type NotifyData struct { Title string Body string diff --git a/socket.go b/socket.go index 6fcba5c..91b37c0 100644 --- a/socket.go +++ b/socket.go @@ -204,6 +204,10 @@ func (i *ITD) Address(_ *server.Context) string { return i.dev.Address() } +func (i *ITD) Navigation(_ *server.Context, data api.NavigationData) error { + return i.dev.Navigation(data.Flag, data.Narrative, data.Dist, data.Progress) +} + func (i *ITD) Notify(_ *server.Context, data api.NotifyData) error { return i.dev.Notify(data.Title, data.Body) } ```
yannickulrich added 6 commits 3 months ago
Arsen6331 requested changes 3 months ago
Arsen6331 left a comment
Owner

Thank you for the PR.

If you have issues completing any of the changes I requested, simply do what you can and I'll handle the rest.

Thank you for the PR. If you have issues completing any of the changes I requested, simply do what you can and I'll handle the rest.
infinitime.go Outdated
NavProgressChar: "Navigation Progress",
}
var NavFlagNames = []string{
Owner

I think it would be better if these were individual constants rather than a slice. That way, they could have their own type and the compiler can guarantee that they're valid instead of needing a separate function. Since that will add quite a bit of code, I think the navigation service can be moved into a separate file.

I think it would be better if these were individual constants rather than a slice. That way, they could have their own type and the compiler can guarantee that they're valid instead of needing a separate function. Since that will add quite a bit of code, I think the navigation service can be moved into a separate file.
Owner

Maybe something like

type NavFlag string

const (
    NavFlagArrive     NavFlag = "arrive"
    NavFlagArriveLeft NavFlag = "arrive-left"
    // ...
)

Then, the NavigationEvent can take a NavFlag instead of string.

Maybe something like ```go type NavFlag string const ( NavFlagArrive NavFlag = "arrive" NavFlagArriveLeft NavFlag = "arrive-left" // ... ) ``` Then, the `NavigationEvent` can take a `NavFlag` instead of `string`.
Poster

Taken care of in d46f545

Taken care of in d46f545
yannickulrich marked this conversation as resolved
infinitime.go Outdated
notifEventDone bool
Music MusicCtrl
DFU DFU
navigationEv NavigationEvent
Owner

I don't see why this is needed. It never gets used except to set its values, so there's no reason for it to be here.

I don't see why this is needed. It never gets used except to set its values, so there's no reason for it to be here.
Poster

The idea here was that we only update a field when it actually changes. Alternatively, the field in NavigationEvent could be nil or something to indicate that we won't want to update this. How would you do this? I don't have that much experience with Go.

The idea here was that we only update a field when it actually changes. Alternatively, the field in NavigationEvent could be `nil` or something to indicate that we won't want to update this. How would you do this? I don't have that much experience with Go.
Owner

I actually think that you don't need to make sure it's changed. GATT characteristic writes are fast enough that it doesn't really cause a problem if you update them with the same value. InfiniTime might even check if it's changed in the navigation service code.

For the music service, for example, I update the values as they come in from the DBus MPRIS interface. This does mean that if you, for example, reload a page, it will send the same values again, but this has never caused a problem because writes take milliseconds.

I actually think that you don't need to make sure it's changed. GATT characteristic writes are fast enough that it doesn't really cause a problem if you update them with the same value. InfiniTime might even check if it's changed in the navigation service code. For the music service, for example, I update the values as they come in from the DBus MPRIS interface. This does mean that if you, for example, reload a page, it will send the same values again, but this has never caused a problem because writes take milliseconds.
infinitime.go Outdated
}
// Navigation sends a NavigationEvent to the watch
func (i *Device) Navigation(flag string, narrative string, dist string, progress uint8) error {
Owner

I'd prefer if this accepted a NavigationEvent rather than individual arguments. That also means all the fields in NavigationEvent should be capitalized to export them.

I'd prefer if this accepted a `NavigationEvent` rather than individual arguments. That also means all the fields in `NavigationEvent` should be capitalized to export them.
Poster

Sure! See c3a8727 but also my comment above

Sure! See c3a8727 but also my comment [above](https://gitea.arsenm.dev/Arsen6331/infinitime/pulls/5#issuecomment-322)
yannickulrich marked this conversation as resolved
yannickulrich added 3 commits 3 months ago
Poster

Changes to the itd daemon with the changes to the API

diff --git a/api/navigation.go b/api/navigation.go
new file mode 100644
index 0000000..809e0c6
--- /dev/null
+++ b/api/navigation.go
@@ -0,0 +1,22 @@
+package api
+
+import (
+    "context"
+       "go.arsenm.dev/infinitime"
+)
+
+func (c *Client) Navigation(ctx context.Context, flag infinitime.NavFlag, narrative string, dist string, progress uint8) error {
+       return c.client.Call(
+               ctx,
+               "ITD",
+               "Navigation",
+               infinitime.NavigationEvent{
+                       Flag: flag,
+                       Narrative: narrative,
+                       Dist: dist,
+                       Progress: progress,
+               },
+               nil,
+       )
+}
+
diff --git a/socket.go b/socket.go
index 6fcba5c..9332afb 100644
--- a/socket.go
+++ b/socket.go
@@ -204,6 +204,10 @@ func (i *ITD) Address(_ *server.Context) string {
        return i.dev.Address()
 }

+func (i *ITD) Navigation(_ *server.Context, data infinitime.NavigationEvent) error {
+       return i.dev.SetNavigation(data)
+}
+
 func (i *ITD) Notify(_ *server.Context, data api.NotifyData) error {
        return i.dev.Notify(data.Title, data.Body)
 }
Changes to the `itd` daemon with the changes to the API ```patch diff --git a/api/navigation.go b/api/navigation.go new file mode 100644 index 0000000..809e0c6 --- /dev/null +++ b/api/navigation.go @@ -0,0 +1,22 @@ +package api + +import ( + "context" + "go.arsenm.dev/infinitime" +) + +func (c *Client) Navigation(ctx context.Context, flag infinitime.NavFlag, narrative string, dist string, progress uint8) error { + return c.client.Call( + ctx, + "ITD", + "Navigation", + infinitime.NavigationEvent{ + Flag: flag, + Narrative: narrative, + Dist: dist, + Progress: progress, + }, + nil, + ) +} + diff --git a/socket.go b/socket.go index 6fcba5c..9332afb 100644 --- a/socket.go +++ b/socket.go @@ -204,6 +204,10 @@ func (i *ITD) Address(_ *server.Context) string { return i.dev.Address() } +func (i *ITD) Navigation(_ *server.Context, data infinitime.NavigationEvent) error { + return i.dev.SetNavigation(data) +} + func (i *ITD) Notify(_ *server.Context, data api.NotifyData) error { return i.dev.Notify(data.Title, data.Body) } ```
yannickulrich requested review from Arsen6331 3 months ago
Arsen6331 approved these changes 3 months ago
Arsen6331 merged commit 1f5301f5de into master 3 months ago
Arsen6331 referenced this issue from a commit 3 months ago
Poster

Thanks for merging 💜
I've also prepared the changes for itd. Do you want me to open a pull request on Arsen6331/itd or do you just want to apply the above patch?

Thanks for merging 💜 I've also prepared the [changes for itd](https://gitea.arsenm.dev/Arsen6331/infinitime/pulls/5#issuecomment-326). Do you want me to open a pull request on [Arsen6331/itd](https://gitea.arsenm.dev/Arsen6331/itd) or do you just want to apply the above patch?
Owner

Thanks for merging 💜
I've also prepared the changes for itd. Do you want me to open a pull request on Arsen6331/itd or do you just want to apply the above patch?

I'll probably just add PureMaps support directly rather than exposing it in the RPC interface, unless you have a specific need for that

> Thanks for merging 💜 > I've also prepared the [changes for itd](https://gitea.arsenm.dev/Arsen6331/infinitime/pulls/5#issuecomment-326). Do you want me to open a pull request on [Arsen6331/itd](https://gitea.arsenm.dev/Arsen6331/itd) or do you just want to apply the above patch? I'll probably just add PureMaps support directly rather than exposing it in the RPC interface, unless you have a specific need for that

Reviewers

Arsen6331 approved these changes 3 months ago
The pull request has been merged as 1f5301f5de.
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Arsen6331/infinitime#5
Loading…
There is no content yet.