Repository: meshtastic/python

Issue:
[Bug]: setting fixed lat/lon/alt now REQUIRES ``--set position.fixed_position true`` even if ALREADY set to True
### Category

Other

### Hardware

Rak4631

### Firmware Version

2.3.1.4fa7f5a

### Description

I used fixed position with previous fw version. after upgrading to this one, I cannot set it via cli nor via android app.

For cli see log output below, for app I set it in gui, reconnect and shows as lat/long/alt of 0.0.

### Relevant log output

```Shell
``
$> meshtastic --setlat 37 --setlon -121 --setalt 30
Connected to radio
Fixing altitude at 30 meters
Fixing latitude at 37.0 degrees
Fixing longitude at -121.0 degrees
Setting device position

$> meshtastic --export-config
...
  position:
    broadcastSmartMinimumDistance: 100
    broadcastSmartMinimumIntervalSecs: 30
    fixedPosition: true
    gpsMode: NOT_PRESENT
    gpsUpdateInterval: 120
    positionBroadcastSecs: 900
    positionFlags: 811
...
```

Discussion:
@saahbs: note: the location section is not present at all!@thebentern: Location is not a part of config. It's stored on the node in the NodeDB of the device@saahbs: As of 2.2.24 to which I downgraded, the fixed position is stored in location: which is NOT nodeDB AFAICT, see:

```
$> meshtastic --export-config
...
location:
  alt: 30
  lat: 37.310116
  lon: -121.88448199999999
...
```

Setting fixed location does NOT work on 2.3.1 at all.@francisuk1989: I had this issue aswell on 2.3.1 alpha however did manage to fix it by turning off the smart control and fix gps in the android app and then used the python ctl to set it.

`meshtastic --host 192.168.2.89 --set position.fixed_position true --setlat XXX --setlon XXXX --setalt XX`

This might work instead of using the app/web browser but not tried it, maybe someone can confirm before re-enabling it again via python ctl ?
`meshtastic --host 192.168.2.89 --set position.fixed_position false`
@saahbs: Interesting, perhaps with 2.3.x one needs to specify fixed_position true and ALL of lat/lon/alt at ONCE for it to take effect. I did ``--export-config`` from 2.2.24, which had ``fixed_position: true`` and all ``location:`` fields set, upgraded to 2.3.1 again and ran ``--configure exported.yaml`` and it took.

Somehow order of operations matters now and this breaks the android app.

ps. I checked my earlier export-config which I used before 2.3.1 upgrade and it DID not have ``location:`` in it (maybe I was experimenting and cleared it - which explains why I ran into this initially and not after I did full dump on 2.2.24 before configure on 2.3.1.@saahbs: moar debugging on 2.3.1:

```
> /meshtastic --setlat 0 --setlon 0 --setalt 0 --set position.fixed_position false
Connected to radio
Fixing altitude at 0 meters
Fixing latitude at 0.0 degrees
Fixing longitude at 0.0 degrees
Setting device position
Set position.fixed_position to false
Writing modified preferences to device
```

but when I --export-config I see:
```
...
  position:
    broadcastSmartMinimumDistance: 100
    broadcastSmartMinimumIntervalSecs: 30
    gpsMode: NOT_PRESENT
    gpsUpdateInterval: 120
    positionBroadcastSecs: 900
    positionFlags: 811
...
location:
  alt: 30
  lat: 37.310116
  lon: -121.88448199999999
```

so clearly the device is not accepting the lat/lon/alt for some reason. THIS is the bug.@saahbs: This does work:

```
> /meshtastic --setlat 1 --setlon 2 --setalt 3 --set position.fixed_position true
Connected to radio
Fixing altitude at 3 meters
Fixing latitude at 1.0 degrees
Fixing longitude at 2.0 degrees
Setting device position
Set position.fixed_position to true
Writing modified preferences to device
```

and --export-config:
```
...
  position:
    broadcastSmartMinimumDistance: 100
    broadcastSmartMinimumIntervalSecs: 30
    fixedPosition: true
    gpsMode: NOT_PRESENT
    gpsUpdateInterval: 120
    positionBroadcastSecs: 900
    positionFlags: 811
...
location:
  alt: 3
  lat: 1.0
  lon: 2.0
...
```@saahbs: Lets try setting lat/lon/alt without passing set:

```
> meshtastic --setlat 4 --setlon 5 --setalt 6
Connected to radio
Fixing altitude at 6 meters
Fixing latitude at 4.0 degrees
Fixing longitude at 5.0 degrees
Setting device position
```

and --export-config:

```
...
  position:
    broadcastSmartMinimumDistance: 100
    broadcastSmartMinimumIntervalSecs: 30
    fixedPosition: true
    gpsMode: NOT_PRESENT
    gpsUpdateInterval: 120
    positionBroadcastSecs: 900
    positionFlags: 811
...
location:
  alt: 3
  lat: 1.0
  lon: 2.0
...
```

FAIL, passing any of --set{lan,lon,alt} now appears to REQUIRE --set position.fixed_position true even if it is ALREADY set to True.

^^^ please fix.@GUVWAF: You now have to disable fixed position first in order to be able to set your position. This was deliberately put in by meshtastic/firmware#3403, because there have been many reports of people that expected the position to never change after you set fixed position to true. This was especially for phone apps, but the CLI uses the same API.@thebentern: We need to update python to send the position packet first and then enable fixed position as a part of this process. There is no reason to use --setlat --setlon etc outside of the context of a fixed position

Solution patch:
diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py
index 77ec148..66e7559 100644
--- a/meshtastic/__main__.py
+++ b/meshtastic/__main__.py
@@ -257,33 +257,41 @@ def onConnected(interface):
         if not args.export_config:
             print("Connected to radio")
 
-        if args.setlat or args.setlon or args.setalt:
+        if args.remove_position:
+            if args.dest != BROADCAST_ADDR:
+                print("Setting positions of remote nodes is not supported.")
+                return
+            closeNow = True
+            print("Removing fixed position and disabling fixed position setting")
+            interface.localNode.removeFixedPosition()
+        elif args.setlat or args.setlon or args.setalt:
             if args.dest != BROADCAST_ADDR:
                 print("Setting latitude, longitude, and altitude of remote nodes is not supported.")
                 return
             closeNow = True
 
             alt = 0
-            lat = 0.0
-            lon = 0.0
-            localConfig = interface.localNode.localConfig
+            lat = 0
+            lon = 0
             if args.setalt:
                 alt = int(args.setalt)
-                localConfig.position.fixed_position = True
                 print(f"Fixing altitude at {alt} meters")
             if args.setlat:
-                lat = float(args.setlat)
-                localConfig.position.fixed_position = True
+                try:
+                    lat = int(args.setlat)
+                except ValueError:
+                    lat = float(args.setlat)
                 print(f"Fixing latitude at {lat} degrees")
             if args.setlon:
-                lon = float(args.setlon)
-                localConfig.position.fixed_position = True
+                try:
+                    lon = int(args.setlon)
+                except ValueError:
+                    lon = float(args.setlon)
                 print(f"Fixing longitude at {lon} degrees")
 
-            print("Setting device position")
+            print("Setting device position and enabling fixed position setting")
             # can include lat/long/alt etc: latitude = 37.5, longitude = -122.1
-            interface.sendPosition(lat, lon, alt)
-            interface.localNode.writeConfig("position")
+            interface.localNode.setFixedPosition(lat, lon, alt)
         elif not args.no_time:
             # We normally provide a current time to the mesh when we connect
             if interface.localNode.nodeNum in interface.nodesByNum and "position" in interface.nodesByNum[interface.localNode.nodeNum]:
@@ -1445,12 +1453,25 @@ def initParser():
         action="store_true",
     )
 
-    group.add_argument("--setalt", help="Set device altitude in meters (allows use without GPS)")
+    group.add_argument(
+        "--setalt",
+        help="Set device altitude in meters (allows use without GPS), and enable fixed position.",
+    )
 
-    group.add_argument("--setlat", help="Set device latitude (allows use without GPS)")
+    group.add_argument(
+        "--setlat",
+        help="Set device latitude (allows use without GPS), and enable fixed position. Accepts a decimal value or an integer premultiplied by 1e7.",
+    )
+
+    group.add_argument(
+        "--setlon",
+        help="Set device longitude (allows use without GPS), and enable fixed position. Accepts a decimal value or an integer premultiplied by 1e7.",
+    )
 
     group.add_argument(
-        "--setlon", help="Set device longitude (allows use without GPS)"
+        "--remove-position",
+        help="Clear any existing fixed position and disable fixed position.",
+        action="store_true",
     )
 
     group.add_argument(
diff --git a/meshtastic/mesh_interface.py b/meshtastic/mesh_interface.py
index 7d7495e..1c24fb9 100644
--- a/meshtastic/mesh_interface.py
+++ b/meshtastic/mesh_interface.py
@@ -9,6 +9,7 @@ import sys
 import threading
 import time
 from datetime import datetime
+from decimal import Decimal
 
 from typing import Any, Callable, Dict, List, Optional, Union
 
@@ -978,9 +979,9 @@ class MeshInterface:
         Returns the position with the updated keys
         """
         if "latitudeI" in position:
-            position["latitude"] = position["latitudeI"] * 1e-7
+            position["latitude"] = float(position["latitudeI"] * Decimal("1e-7"))
         if "longitudeI" in position:
-            position["longitude"] = position["longitudeI"] * 1e-7
+            position["longitude"] = float(position["longitudeI"] * Decimal("1e-7"))
         return position
 
     def _nodeNumToId(self, num):
diff --git a/meshtastic/node.py b/meshtastic/node.py
index a7fb191..277f3b9 100644
--- a/meshtastic/node.py
+++ b/meshtastic/node.py
@@ -7,7 +7,7 @@ import time
 
 from typing import Union
 
-from meshtastic import admin_pb2, apponly_pb2, channel_pb2, localonly_pb2, portnums_pb2
+from meshtastic import admin_pb2, apponly_pb2, channel_pb2, localonly_pb2, mesh_pb2, portnums_pb2
 from meshtastic.util import (
     Timeout,
     camel_to_snake,
@@ -655,6 +655,38 @@ class Node:
             onResponse = self.onAckNak
         return self._sendAdmin(p, onResponse=onResponse)
 
+    def setFixedPosition(self, lat: Union[int, float], lon: Union[int, float], alt: int):
+        """Tell the node to set fixed position to the provided value and enable the fixed position setting"""
+        if self != self.iface.localNode:
+            logging.error("Setting position of remote nodes is not supported.")
+            return None
+
+        p = mesh_pb2.Position()
+        if isinstance(lat, float) and lat != 0.0:
+            p.latitude_i = int(lat / 1e-7)
+        elif isinstance(lat, int) and lat != 0:
+            p.latitude_i = lat
+
+        if isinstance(lon, float) and lon != 0.0:
+            p.longitude_i = int(lon / 1e-7)
+        elif isinstance(lon, int) and lon != 0:
+            p.longitude_i = lon
+
+        if alt != 0:
+            p.altitude = alt
+
+        a = admin_pb2.AdminMessage()
+        a.set_fixed_position.CopyFrom(p)
+        return self._sendAdmin(a)
+
+    def removeFixedPosition(self):
+        """Tell the node to remove the fixed position and set the fixed position setting to false"""
+        p = admin_pb2.AdminMessage()
+        p.remove_fixed_position = True
+        logging.info(f"Telling node to remove fixed position")
+
+        return self._sendAdmin(p)
+
     def _fixupChannels(self):
         """Fixup indexes and add disabled channels as needed"""
 


Test patch:
diff --git a/meshtastic/tests/test_main.py b/meshtastic/tests/test_main.py
index dba635b..a5a345a 100644
--- a/meshtastic/tests/test_main.py
+++ b/meshtastic/tests/test_main.py
@@ -734,19 +734,14 @@ def test_main_setlat(capsys):
 
     mocked_node = MagicMock(autospec=Node)
 
-    def mock_writeConfig():
-        print("inside mocked writeConfig")
+    def mock_setFixedPosition(lat, lon, alt):
+        print("inside mocked setFixedPosition")
+        print(f"{lat} {lon} {alt}")
 
-    mocked_node.writeConfig.side_effect = mock_writeConfig
+    mocked_node.setFixedPosition.side_effect = mock_setFixedPosition
 
     iface = MagicMock(autospec=SerialInterface)
-
-    def mock_sendPosition(lat, lon, alt):
-        print("inside mocked sendPosition")
-        print(f"{lat} {lon} {alt}")
-
-    iface.sendPosition.side_effect = mock_sendPosition
-    iface.localNode.return_value = mocked_node
+    iface.localNode = mocked_node
 
     with patch("meshtastic.serial_interface.SerialInterface", return_value=iface) as mo:
         main()
@@ -754,8 +749,7 @@ def test_main_setlat(capsys):
         assert re.search(r"Connected to radio", out, re.MULTILINE)
         assert re.search(r"Fixing latitude", out, re.MULTILINE)
         assert re.search(r"Setting device position", out, re.MULTILINE)
-        assert re.search(r"inside mocked sendPosition", out, re.MULTILINE)
-        # TODO: Why does this not work? assert re.search(r'inside mocked writeConfig', out, re.MULTILINE)
+        assert re.search(r"inside mocked setFixedPosition", out, re.MULTILINE)
         assert err == ""
         mo.assert_called()
 
@@ -769,19 +763,14 @@ def test_main_setlon(capsys):
 
     mocked_node = MagicMock(autospec=Node)
 
-    def mock_writeConfig():
-        print("inside mocked writeConfig")
+    def mock_setFixedPosition(lat, lon, alt):
+        print("inside mocked setFixedPosition")
+        print(f"{lat} {lon} {alt}")
 
-    mocked_node.writeConfig.side_effect = mock_writeConfig
+    mocked_node.setFixedPosition.side_effect = mock_setFixedPosition
 
     iface = MagicMock(autospec=SerialInterface)
-
-    def mock_sendPosition(lat, lon, alt):
-        print("inside mocked sendPosition")
-        print(f"{lat} {lon} {alt}")
-
-    iface.sendPosition.side_effect = mock_sendPosition
-    iface.localNode.return_value = mocked_node
+    iface.localNode = mocked_node
 
     with patch("meshtastic.serial_interface.SerialInterface", return_value=iface) as mo:
         main()
@@ -789,8 +778,7 @@ def test_main_setlon(capsys):
         assert re.search(r"Connected to radio", out, re.MULTILINE)
         assert re.search(r"Fixing longitude", out, re.MULTILINE)
         assert re.search(r"Setting device position", out, re.MULTILINE)
-        assert re.search(r"inside mocked sendPosition", out, re.MULTILINE)
-        # TODO: Why does this not work? assert re.search(r'inside mocked writeConfig', out, re.MULTILINE)
+        assert re.search(r"inside mocked setFixedPosition", out, re.MULTILINE)
         assert err == ""
         mo.assert_called()
 
@@ -804,19 +792,14 @@ def test_main_setalt(capsys):
 
     mocked_node = MagicMock(autospec=Node)
 
-    def mock_writeConfig():
-        print("inside mocked writeConfig")
+    def mock_setFixedPosition(lat, lon, alt):
+        print("inside mocked setFixedPosition")
+        print(f"{lat} {lon} {alt}")
 
-    mocked_node.writeConfig.side_effect = mock_writeConfig
+    mocked_node.setFixedPosition.side_effect = mock_setFixedPosition
 
     iface = MagicMock(autospec=SerialInterface)
-
-    def mock_sendPosition(lat, lon, alt):
-        print("inside mocked sendPosition")
-        print(f"{lat} {lon} {alt}")
-
-    iface.sendPosition.side_effect = mock_sendPosition
-    iface.localNode.return_value = mocked_node
+    iface.localNode = mocked_node
 
     with patch("meshtastic.serial_interface.SerialInterface", return_value=iface) as mo:
         main()
@@ -824,8 +807,7 @@ def test_main_setalt(capsys):
         assert re.search(r"Connected to radio", out, re.MULTILINE)
         assert re.search(r"Fixing altitude", out, re.MULTILINE)
         assert re.search(r"Setting device position", out, re.MULTILINE)
-        assert re.search(r"inside mocked sendPosition", out, re.MULTILINE)
-        # TODO: Why does this not work? assert re.search(r'inside mocked writeConfig', out, re.MULTILINE)
+        assert re.search(r"inside mocked setFixedPosition", out, re.MULTILINE)
         assert err == ""
         mo.assert_called()