Skip to content

Conversation

functionpointer
Copy link
Contributor

@functionpointer functionpointer commented Sep 28, 2025

User description

Enables FrSky SmartPort sensors like FAS100 or UniSens-E to be used as battery sensors for iNav.

Usage:

  1. Connect SmartPort sensor to a UART TX
  2. In the Ports tab, enable SmartPort Master on that UART
  3. In the Configuration tab, choose SMARTPORT as Voltage and/or Current Meter Type
grafik

I have tested:

  • Current sensing using UniSens-E
  • Voltage sensing using UniSens-E
  • Readings drop to 0 when sensor loses power
  • Readings come back when sensor recovers power
  • Readings verified against a lab power supply

PR Type

Enhancement


Description

This description is generated by an AI tool. It may have inaccuracies

  • Add SmartPort battery sensor support for voltage/current

  • Enable FrSky SmartPort sensors as battery meters

  • Add new sensor types SMARTPORT for configuration

  • Implement data decoding and timeout handling


Diagram Walkthrough

flowchart LR
  A["SmartPort Sensor"] -- "VFAS/Current Data" --> B["SmartPort Master"]
  B -- "Decoded Values" --> C["Battery System"]
  C --> D["Voltage Meter"]
  C --> E["Current Meter"]
Loading

File Walkthrough

Relevant files
Enhancement
smartport_master.c
SmartPort voltage and current sensor implementation           

src/main/io/smartport_master.c

  • Add voltage and current data fields to sensor structure
  • Implement decoding functions for VFAS and current data
  • Add getter functions for voltage and current with timeout checks
  • Fix timeout constant from milliseconds to microseconds
+44/-1   
smartport_master.h
SmartPort battery sensor API declarations                               

src/main/io/smartport_master.h

  • Add function declarations for voltage and current data getters
  • Add batteryData_t structure definition
+7/-0     
battery.c
Battery system SmartPort sensor integration                           

src/main/sensors/battery.c

  • Add SmartPort voltage sensor case in updateBatteryVoltage
  • Add SmartPort current sensor case in currentMeterUpdate
  • Include smartport_master.h header
+24/-1   
battery_config_structs.h
Battery sensor configuration enum updates                               

src/main/sensors/battery_config_structs.h

  • Add CURRENT_SENSOR_SMARTPORT and VOLTAGE_SENSOR_SMARTPORT enums
  • Update VOLTAGE_SENSOR_MAX constant
+4/-1     
Documentation
Settings.md
Documentation update for SmartPort sensor                               

docs/Settings.md

  • Update current_meter_type documentation to include SMARTPORT option
+1/-1     
Configuration changes
settings.yaml
Settings configuration for SmartPort sensors                         

src/main/fc/settings.yaml

  • Add SMARTPORT to current_sensor and voltage_sensor value tables
  • Update configuration descriptions to mention SmartPort requirements
+4/-4     

Voltage and current sensors from smartport master
can now be used as voltage and current sensors for inav
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
⚡ Recommended focus areas for review

Unit/Scaling

Confirm that the decoded units for current (100 mA steps -> assigned as value*10) and voltage (VFAS) match the internal expectations of the battery subsystem (mA vs 0.01A, mV vs 0.1V). A mismatch could skew readings and alarms.

static void decodeCurrentData(uint32_t sdata)
{
    // data comes in 100mA steps
    sensorsData.current = ((int16_t)sdata) * 10;
}

static void decodeVoltageData(uint32_t sdata)
{
    sensorsData.voltage = (int16_t)sdata;
}

static void processSensorPayload(smartPortPayload_t *payload, timeUs_t currentTimeUs)
Enum Consistency

CURRENT_SENSOR_MAX and VOLTAGE_SENSOR_MAX constants appear inconsistent after adding SMARTPORT; verify that MAX values cover the new entries and are used correctly across the codebase.

typedef enum {
    CURRENT_SENSOR_NONE = 0,
    CURRENT_SENSOR_ADC,
    CURRENT_SENSOR_VIRTUAL,
    CURRENT_SENSOR_FAKE,
    CURRENT_SENSOR_ESC,
    CURRENT_SENSOR_SMARTPORT,
    CURRENT_SENSOR_MAX = CURRENT_SENSOR_FAKE
} currentSensor_e;

typedef enum {
    VOLTAGE_SENSOR_NONE = 0,
    VOLTAGE_SENSOR_ADC,
    VOLTAGE_SENSOR_ESC,
    VOLTAGE_SENSOR_FAKE,
    VOLTAGE_SENSOR_SMARTPORT,
    VOLTAGE_SENSOR_MAX = VOLTAGE_SENSOR_FAKE
} voltageSensor_e;
Shadowing/Scope

Local pointer variables smartportData are reused in separate switch cases; while scoped, the shared name could reduce readability. Consider distinct names (spVoltage, spCurrent) to avoid confusion.

#if defined(USE_SMARTPORT_MASTER)
    case VOLTAGE_SENSOR_SMARTPORT:
        int16_t * smartportData = smartportMasterGetVoltageData();
        if (smartportData) {
            vbat = *smartportData;
        } else {
            vbat = 0;
        }
        break;
#endif

Copy link

qodo-merge-pro bot commented Sep 28, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Fix incorrect sensor enum max values

The CURRENT_SENSOR_MAX and VOLTAGE_SENSOR_MAX enum values in
battery_config_structs.h are incorrect. They must be updated to include the
newly added SMARTPORT sensor types to prevent validation failures and other
bugs.

Examples:

src/main/sensors/battery_config_structs.h [28-36]
typedef enum {
    CURRENT_SENSOR_NONE = 0,
    CURRENT_SENSOR_ADC,
    CURRENT_SENSOR_VIRTUAL,
    CURRENT_SENSOR_FAKE,
    CURRENT_SENSOR_ESC,
    CURRENT_SENSOR_SMARTPORT,
    CURRENT_SENSOR_MAX = CURRENT_SENSOR_FAKE
} currentSensor_e;
src/main/sensors/battery_config_structs.h [38-45]
typedef enum {
    VOLTAGE_SENSOR_NONE = 0,
    VOLTAGE_SENSOR_ADC,
    VOLTAGE_SENSOR_ESC,
    VOLTAGE_SENSOR_FAKE,
    VOLTAGE_SENSOR_SMARTPORT,
    VOLTAGE_SENSOR_MAX = VOLTAGE_SENSOR_FAKE
} voltageSensor_e;

Solution Walkthrough:

Before:

// src/main/sensors/battery_config_structs.h
typedef enum {
    CURRENT_SENSOR_NONE = 0,
    ...,
    CURRENT_SENSOR_ESC,
    CURRENT_SENSOR_SMARTPORT,
    CURRENT_SENSOR_MAX = CURRENT_SENSOR_FAKE // Incorrect
} currentSensor_e;

typedef enum {
    VOLTAGE_SENSOR_NONE = 0,
    ...,
    VOLTAGE_SENSOR_FAKE,
    VOLTAGE_SENSOR_SMARTPORT,
    VOLTAGE_SENSOR_MAX = VOLTAGE_SENSOR_FAKE // Incorrect
} voltageSensor_e;

After:

// src/main/sensors/battery_config_structs.h
typedef enum {
    CURRENT_SENSOR_NONE = 0,
    ...,
    CURRENT_SENSOR_ESC,
    CURRENT_SENSOR_SMARTPORT,
    CURRENT_SENSOR_MAX = CURRENT_SENSOR_SMARTPORT // Correct
} currentSensor_e;

typedef enum {
    VOLTAGE_SENSOR_NONE = 0,
    ...,
    VOLTAGE_SENSOR_FAKE,
    VOLTAGE_SENSOR_SMARTPORT,
    VOLTAGE_SENSOR_MAX = VOLTAGE_SENSOR_SMARTPORT // Correct
} voltageSensor_e;
Suggestion importance[1-10]: 9

__

Why: This is a critical bug fix; failing to update the _MAX values in the enums breaks range checks and validation, which could cause incorrect behavior or prevent the new sensor types from being used.

High
Possible issue
Update max enum values
Suggestion Impact:The commit updated both enums so that *_MAX now equals *_SMARTPORT, matching the suggestion to reflect the new maximum values.

code diff:

-    CURRENT_SENSOR_MAX = CURRENT_SENSOR_FAKE
+    CURRENT_SENSOR_MAX = CURRENT_SENSOR_SMARTPORT
 } currentSensor_e;
 
 typedef enum {
@@ -41,7 +41,7 @@
     VOLTAGE_SENSOR_ESC,
     VOLTAGE_SENSOR_FAKE,
     VOLTAGE_SENSOR_SMARTPORT,
-    VOLTAGE_SENSOR_MAX = VOLTAGE_SENSOR_FAKE
+    VOLTAGE_SENSOR_MAX = VOLTAGE_SENSOR_SMARTPORT

Update CURRENT_SENSOR_MAX and VOLTAGE_SENSOR_MAX in their respective enums to
CURRENT_SENSOR_SMARTPORT to correctly reflect the new maximum value.

src/main/sensors/battery_config_structs.h [28-45]

 typedef enum {
     CURRENT_SENSOR_NONE = 0,
     CURRENT_SENSOR_ADC,
     CURRENT_SENSOR_VIRTUAL,
     CURRENT_SENSOR_FAKE,
     CURRENT_SENSOR_ESC,
     CURRENT_SENSOR_SMARTPORT,
-    CURRENT_SENSOR_MAX = CURRENT_SENSOR_FAKE
+    CURRENT_SENSOR_MAX = CURRENT_SENSOR_SMARTPORT
 } currentSensor_e;
 
 typedef enum {
     VOLTAGE_SENSOR_NONE = 0,
     VOLTAGE_SENSOR_ADC,
     VOLTAGE_SENSOR_ESC,
     VOLTAGE_SENSOR_FAKE,
     VOLTAGE_SENSOR_SMARTPORT,
-    VOLTAGE_SENSOR_MAX = VOLTAGE_SENSOR_FAKE
+    VOLTAGE_SENSOR_MAX = VOLTAGE_SENSOR_SMARTPORT
 } voltageSensor_e;

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a bug where the _MAX values in two enums were not updated after adding new members, which could lead to out-of-bounds memory access.

High
Avoid resetting voltage on timeout

Avoid resetting the battery voltage vbat to 0 when SmartPort data is
unavailable. Instead, retain the last known voltage value to prevent false
failsafe triggers.

src/main/sensors/battery.c [298-307]

 #if defined(USE_SMARTPORT_MASTER)
 case VOLTAGE_SENSOR_SMARTPORT:
-    int16_t * smartportData = smartportMasterGetVoltageData();
-    if (smartportData) {
-        vbat = *smartportData;
-    } else {
-        vbat = 0;
+    {
+        int16_t * smartportData = smartportMasterGetVoltageData();
+        if (smartportData) {
+            vbat = *smartportData;
+        }
     }
     break;
 #endif
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly points out that resetting vbat to 0 on data loss can cause unwanted failsafe triggers, and proposes a more robust solution by retaining the last known value.

Medium
General
Avoid resetting current on timeout

Avoid resetting amperage to 0 when SmartPort data is unavailable. Instead,
retain the last known value to ensure more accurate mAhDrawn calculations during
sensor dropouts.

src/main/sensors/battery.c [615-624]

 #if defined(USE_SMARTPORT_MASTER)
         case CURRENT_SENSOR_SMARTPORT:
-            int16_t * smartportData = smartportMasterGetCurrentData();
-            if (smartportData) {
-                amperage = *smartportData;
-            } else {
-                amperage = 0;
+            {
+                int16_t * smartportData = smartportMasterGetCurrentData();
+                if (smartportData) {
+                    amperage = *smartportData;
+                }
             }
             break;
 #endif
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that resetting amperage to 0 on data loss leads to inaccurate capacity calculations and proposes a more robust approach by retaining the last known value.

Medium
  • Update

@sensei-hacker
Copy link
Collaborator

sensei-hacker commented Sep 29, 2025

Thanks for submitting this, and for explicitly listing the testing you did!

@sensei-hacker sensei-hacker merged commit a97fb70 into iNavFlight:master Oct 12, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants