Elara6331
/
infinitime
Archived
2
0
Fork 4
Code Issues Pull Requests Projects Releases Wiki Activity

Added Navigation service #5

Merged
Elara6331 merged 9 commits from yannickulrich/infinitime:navigation-service into master 2022-11-03 19:09:07 +00:00
Conversation 3 Commits 9 Files Changed 3 +205
yannickulrich commented 2022-10-21 22:26:32 +00:00
Contributor
Copy Link

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 2022-10-21 22:26:32 +00:00
Elara6331 requested changes 2022-10-21 22:36:24 +00:00
Elara6331 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
@ -55,0 +62,4 @@
NavProgressChar: "Navigation Progress",
}
var NavFlagNames = []string{
Elara6331 commented 2022-10-21 22:34:57 +00:00
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.
Elara6331 commented 2022-10-21 22:41:38 +00:00
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`.
yannickulrich commented 2022-10-22 12:57:06 +00:00
Author
Contributor

Taken care of in d46f545

Taken care of in d46f545
yannickulrich marked this conversation as resolved
infinitime.go Outdated
@ -71,6 +125,7 @@ type Device struct {
notifEventDone bool
Music MusicCtrl
DFU DFU
navigationEv NavigationEvent
Elara6331 commented 2022-10-21 22:32:23 +00:00
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.
yannickulrich commented 2022-10-22 12:56:05 +00:00
Author
Contributor

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.
Elara6331 commented 2022-10-22 17:49:14 +00:00
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
@ -726,0 +798,4 @@
}
// Navigation sends a NavigationEvent to the watch
func (i *Device) Navigation(flag string, narrative string, dist string, progress uint8) error {
Elara6331 commented 2022-10-21 22:33:24 +00:00
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.
yannickulrich commented 2022-10-22 12:58:02 +00:00
Author
Contributor

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 2022-10-22 12:56:28 +00:00
yannickulrich commented 2022-10-22 12:58:56 +00:00
Author
Contributor
Copy Link

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 Elara6331 2022-10-22 12:59:05 +00:00
Elara6331 approved these changes 2022-11-03 19:08:30 +00:00
Elara6331 merged commit 1f5301f5de into master 2022-11-03 19:09:07 +00:00
yannickulrich commented 2022-11-05 20:16:19 +00:00
Author
Contributor
Copy Link

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?
Elara6331 commented 2022-11-05 20:17:41 +00:00
Owner
Copy Link

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
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
Elara6331
Labels
Clear labels   
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Elara6331/infinitime#5
No description provided.
Delete Branch "yannickulrich/infinitime:navigation-service"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?