Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new DSHOT debug modes #3262

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -6759,8 +6759,12 @@
"message": "Bidirectional DShot (requires supported ESC firmware)",
"description": "Feature for the ESC/Motor"
},
"configurationDshotEdt": {
"message": "Built-in sensors",
"description": "ESC/Motor feature"
},
"configurationDshotBidirHelp": {
"message": "Sends ESC data to the FC via DShot telemetry. Required by RPM Filtering and dynamic idle. <br> <br>Note: Requires a compatible ESC with appropriate firmware, eg JESC, Jazzmac, BLHeli-32.",
"message": "Sends ESC data to the FC via DShot telemetry. Required by RPM Filtering and dynamic idle. <br> <br>Note: Requires a compatible ESC with appropriate firmware, eg Bluejay, AM32, JESC, Jazzmac, BLHeli-32.",
"description": "Description of the Bidirectional DShot feature of the ESC/Motor"
},
"configurationGyroSyncDenom": {
Expand Down
6 changes: 3 additions & 3 deletions src/css/tabs/onboard_logging.less
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@
.blackboxRate {
select {
float: left;
width: 180px;
width: 260px;
height: 20px;
margin: 0 10px 5px 0;
border: 1px solid var(--subtleAccent);
Expand Down Expand Up @@ -202,7 +202,7 @@
.blackboxDebugMode {
select {
float: left;
width: 180px;
width: 260px;
height: 20px;
margin: 0 10px 5px 0;
border: 1px solid var(--subtleAccent);
Expand All @@ -215,7 +215,7 @@
.blackboxDevice {
select {
float: left;
width: 180px;
width: 260px;
height: 20px;
margin: 0 10px 5px 0;
border: 1px solid var(--subtleAccent);
Expand Down
1 change: 1 addition & 0 deletions src/js/VirtualFC.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const VirtualFC = {
motor_count: 4,
motor_poles: 14,
use_dshot_telemetry: true,
use_dshot_edt: true,
use_esc_sensor: false,
};

Expand Down
7 changes: 7 additions & 0 deletions src/js/debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ const DEBUG = {
{text: "MAG_CALIB"},
{text: "MAG_TASK_RATE"},
{text: "EZLANDING"},
{text: "DSHOT_STATUS_N_TEMPERATURE"},
{text: "DSHOT_STATUS_N_VOLTAGE"},
{text: "DSHOT_STATUS_N_CURRENT"},
{text: "DSHOT_STATUS_N_DEBUG1"},
{text: "DSHOT_STATUS_N_DEBUG2"},
{text: "DSHOT_STATUS_N_STRESS_LVL"},
{text: "DSHOT_STATUS_N_ERPM_FRACTION_18"},
],

fieldNames : {
Expand Down
1 change: 1 addition & 0 deletions src/js/fc.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ const FC = {
motor_count: 0,
motor_poles: 0,
use_dshot_telemetry: false,
use_dshot_edt: false,
use_esc_sensor: false,
};

Expand Down
6 changes: 6 additions & 0 deletions src/js/msp/MSPHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,9 @@ MspHelper.prototype.process_data = function(dataHandler) {
FC.MOTOR_CONFIG.use_dshot_telemetry = data.readU8() != 0;
FC.MOTOR_CONFIG.use_esc_sensor = data.readU8() != 0;
}
if (semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_46)) {
FC.MOTOR_CONFIG.use_dshot_edt = data.readU8() != 0;
}
break;
case MSPCodes.MSP_GPS_CONFIG:
FC.GPS_CONFIG.provider = data.readU8();
Expand Down Expand Up @@ -1799,6 +1802,9 @@ MspHelper.prototype.crunch = function(code, modifierCode = undefined) {
buffer.push8(FC.MOTOR_CONFIG.motor_poles);
buffer.push8(FC.MOTOR_CONFIG.use_dshot_telemetry ? 1 : 0);
}
if (semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_46)) {
buffer.push8(FC.MOTOR_CONFIG.use_dshot_edt ? 1 : 0);
}
break;
case MSPCodes.MSP_SET_GPS_CONFIG:
buffer.push8(FC.GPS_CONFIG.provider)
Expand Down
37 changes: 35 additions & 2 deletions src/js/tabs/motors.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import FC from "../fc";
import MSP from "../msp";
import { mixerList } from "../model";
import MSPCodes from "../msp/MSPCodes";
import { API_VERSION_1_42, API_VERSION_1_44 } from "../data_storage";
import { API_VERSION_1_42, API_VERSION_1_44, API_VERSION_1_46 } from "../data_storage";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@haslinghuis , beside 1_47, what are any suggestions about this line considering some old semvers were removed and considering PWA?
is this line needed at all?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API_VERSION_1_47 needs to be added instead.

import EscProtocols from "../utils/EscProtocols";
import { updateTabList } from "../utils/updateTabList";
import { isInt, getMixerImageSrc } from "../utils/common";
Expand All @@ -22,6 +22,7 @@ import $ from 'jquery';

const motors = {
previousDshotBidir: null,
previousDshotEdt: null,
previousFilterDynQ: null,
previousFilterDynCount: null,
analyticsChanges: {},
Expand Down Expand Up @@ -279,6 +280,7 @@ motors.initialize = async function (callback) {
feature12: FC.FEATURE_CONFIG.features.isEnabled('3D'),
feature27: FC.FEATURE_CONFIG.features.isEnabled('ESC_SENSOR'),
dshotBidir: FC.MOTOR_CONFIG.use_dshot_telemetry,
dshotEdt: FC.MOTOR_CONFIG.use_dshot_edt,
motorPoles: FC.MOTOR_CONFIG.motor_poles,
digitalIdlePercent: FC.PID_ADVANCED_CONFIG.digitalIdlePercent,
idleMinRpm: FC.ADVANCED_TUNING.idleMinRpm,
Expand Down Expand Up @@ -700,6 +702,7 @@ motors.initialize = async function (callback) {
dshotBidirElement.prop('checked', FC.MOTOR_CONFIG.use_dshot_telemetry).trigger("change");

self.previousDshotBidir = FC.MOTOR_CONFIG.use_dshot_telemetry;
self.previousDshotEdt = FC.MOTOR_CONFIG.use_dshot_edt;
self.previousFilterDynQ = FC.FILTER_CONFIG.dyn_notch_q;
self.previousFilterDynCount = FC.FILTER_CONFIG.dyn_notch_count;

Expand All @@ -708,6 +711,12 @@ motors.initialize = async function (callback) {
const newValue = (value !== FC.MOTOR_CONFIG.use_dshot_telemetry) ? 'On' : 'Off';
self.analyticsChanges['BidirectionalDshot'] = newValue;
FC.MOTOR_CONFIG.use_dshot_telemetry = value;
if (semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_46) && !value) {
const dshotEdtElement = $('input[id="dshotEdt"]');

FC.MOTOR_CONFIG.use_dshot_edt = value;
dshotEdtElement.prop('checked', FC.MOTOR_CONFIG.use_dshot_edt).trigger("change");
}

if (semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_44)) {
const rpmFilterIsDisabled = FC.FILTER_CONFIG.gyro_rpm_notch_harmonics === 0;
Expand Down Expand Up @@ -742,6 +751,19 @@ motors.initialize = async function (callback) {
}
});

if (semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_46)) {
const dshotEdtElement = $('input[id="dshotEdt"]');

dshotEdtElement.prop('checked', FC.MOTOR_CONFIG.use_dshot_edt).trigger("change");
dshotEdtElement.on("change", function () {
const value = dshotEdtElement.is(':checked');
const newValue = (value !== FC.MOTOR_CONFIG.use_dshot_edt) ? 'On' : 'Off';

self.analyticsChanges['DshotEdt'] = newValue;
FC.MOTOR_CONFIG.use_dshot_edt = value;
});
Comment on lines +755 to +764
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the code style here.

Better to use this or similar way to get the info about the actual order, sometimes things get weird with closures in js.

Also, not sure what triggering change event and then adding on handler does here 🤔

Something like the following is a bit more streamlined/usual in js land

Suggested change
const dshotEdtElement = $('input[id="dshotEdt"]');
dshotEdtElement.prop('checked', FC.MOTOR_CONFIG.use_dshot_edt).trigger("change");
dshotEdtElement.on("change", function () {
const value = dshotEdtElement.is(':checked');
const newValue = (value !== FC.MOTOR_CONFIG.use_dshot_edt) ? 'On' : 'Off';
self.analyticsChanges['DshotEdt'] = newValue;
FC.MOTOR_CONFIG.use_dshot_edt = value;
});
const dshotEditCheckbox = $('#dshotEdt');
dshotEditCheckbox.prop('checked', FC.MOTOR_CONFIG.use_dshot_edt)
.on("change", function () {
const isChecked = $(this).is(':checked');
const newValue = isChecked !== FC.MOTOR_CONFIG.use_dshot_edt ? 'On' : 'Off';
self.analyticsChanges['DshotEdt'] = newValue;
FC.MOTOR_CONFIG.use_dshot_edt = isChecked;
})
.trigger("change");

}

$('input[name="motorPoles"]').val(FC.MOTOR_CONFIG.motor_poles);
}

Expand Down Expand Up @@ -780,14 +802,20 @@ motors.initialize = async function (callback) {
$('div.digitalIdlePercent').hide();
}

$('.escSensor').toggle(protocolConfigured && digitalProtocol);
$('.escSensor').toggle(
protocolConfigured && digitalProtocol && (
semver.lt(FC.CONFIG.apiVersion, API_VERSION_1_46) ||
semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_46) && !FC.MOTOR_CONFIG.use_dshot_edt));

$('div.checkboxDshotBidir').toggle(protocolConfigured && semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_42) && digitalProtocol);
$('div.motorPoles').toggle(protocolConfigured && rpmFeaturesVisible && semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_42));

$('.escMotorStop').toggle(protocolConfigured);

$('#escProtocolDisabled').toggle(!protocolConfigured);
$('.checkboxDshotEdt').toggle(
protocolConfigured && digitalProtocol && dshotBidirElement.is(':checked') &&
!$("input[name='ESC_SENSOR']").is(':checked') && semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_46));

//trigger change unsyncedPWMSwitch to show/hide Motor PWM freq input
unsyncedPWMSwitchElement.trigger("change");
Expand All @@ -809,6 +837,11 @@ motors.initialize = async function (callback) {

//trigger change dshotBidir and ESC_SENSOR to show/hide Motor Poles tab
dshotBidirElement.change(updateVisibility).trigger("change");
if (semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_46)) {
const dshotEdtElement = $('input[id="dshotEdt"]');

dshotEdtElement.change(updateVisibility).trigger("change");
}
$("input[name='ESC_SENSOR']").on("change", updateVisibility).trigger("change");

// fill throttle
Expand Down
6 changes: 5 additions & 1 deletion src/tabs/motors.html
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,12 @@
<div class="number checkboxDshotBidir">
<div style="float: left; height: 20px; margin-right: 14px; margin-left: 3px;">
<input type="checkbox" id="dshotBidir" class="toggle" />
<span class="freelabel" i18n="configurationDshotBidir"></span>
</div>
<div style="float: left; height: 20px; margin-right: 14px; margin-left: 3px;" class="checkboxDshotEdt">
<input type="checkbox" id="dshotEdt" class="toggle" />
<span class="freelabel" id="dshotEdtLabel" i18n="configurationDshotEdt"></span>
Comment on lines +94 to +96
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooof, these floats look painful 🫠 But probably better for style consistency. More of a note to self/team 🙈

</div>
<span class="freelabel" i18n="configurationDshotBidir"></span>
<div class="helpicon cf_tip" i18n_title="configurationDshotBidirHelp"></div>
</div>
<div class="number motorPoles">
Expand Down