Spindle load feedback with multiple spindles

[dresco]

Am going to have another look at spindle load reporting for VFD spindles, this time just as a percentage (following discussion in previous PR, and in line with how it's done on other machines).

Am planning to query the maximum amps during spindle configuration, and then the actual amps periodically along with the rpm. Can then use a calculated load percentage for feedback in the realtime report etc. So far so good..

I have a question though, I only want to add this additional report data if the spindle is selected. For instance if I switch away from the Huanyang spindle to the PWM spindle with M104, then I'd want to stop adding the data in the Huanyang plugin realtime report. Is there a way within the spindle plugin to determine whether the spindle is currently active?

[terjeio]

Hook your code into the grbl.on_spindle_select event?

[dresco]

Ah yes thanks, so I assume I should just be able to filter on (for instance) the v1_active flag in the realtime report?

But on checking, something looks off... v1_active is set false when the spindle is Huanyang and true when PWM?

Building with VFD_ENABLE = -1, and N_SPINDLE = 2

$395: Default spindle:
    0 - PWM
    1 - Huanyang v1

[terjeio]

Oops - a bug in the registration code, change to:

spindle_id_t spindle_register (const spindle_ptrs_t *spindle, const char *name)
{
    if(n_spindle == 0)
        memcpy(&hal.spindle, spindle, sizeof(spindle_ptrs_t));

    if(n_spindle < N_SPINDLE && settings_add_spindle_type(name)) {
        spindles[n_spindle++] = spindle;
        return n_spindle - 1;
    }

    return -1;
}

The idea behind having a wrapper for VFD registration was to have a place to add common VFD extension code, e.g. by adding VFD specific function pointers by wrapping the "standard" registration struct in a super struct together with a VFD specific struct. The registration function above would then pass on the "standard" struct to the core and save the VFD specific one to the spindles array. By subscribing to the on_spindle_select event in vfd/spindle.c it would become easy to add VFD specific functionality? Perhaps this is what you want?

[dresco]

Thanks, this is what I've got so far.. If you're generally in agreement with the approach, I can open a PR for discussion?

diff --git a/vfd/huanyang.c b/vfd/huanyang.c
index 333dd1f..f150f5b 100644
--- a/vfd/huanyang.c
+++ b/vfd/huanyang.c
@@ -39,6 +39,8 @@ static on_report_options_ptr on_report_options;
 static on_spindle_select_ptr on_spindle_select;
 static driver_reset_ptr driver_reset;
 static uint32_t rpm_max = 0;
+static float amps_max = 0;
+static float amps = 0;
 
 static void rx_exception (uint8_t code, void *context);
 static spindle_data_t *spindleGetData (spindle_data_request_t request);
@@ -84,6 +86,25 @@ static void v1_spindleGetMaxRPM (void)
     modbus_send(&cmd, &v1_callbacks, true);
 }
 
+// Read maximum configured current from spindle, value is used later for calculating spindle load
+static void v1_spindleGetMaxAmps (void)
+{
+    modbus_message_t cmd = {
+        .context = (void *)VFD_GetMaxAmps,
+        .adu[0] = vfd_config.modbus_address,
+        .adu[1] = ModBus_ReadCoils,
+        .adu[2] = 0x03,
+        .adu[3] = 0x8E, // PD142
+        .adu[4] = 0x00,
+        .adu[5] = 0x00,
+        .tx_length = 8,
+        .rx_length = 8
+    };
+
+    modbus_set_silence(&v1_silence);
+    modbus_send(&cmd, &v1_callbacks, true);
+}
+
 static void v1_spindleSetRPM (float rpm, bool block)
 {
     if (rpm != rpm_programmed) {
@@ -151,7 +172,7 @@ static spindle_data_t *v1_spindleGetData (spindle_data_request_t request)
 // Returns spindle state in a spindle_state_t variable
 static spindle_state_t v1_spindleGetState (void)
 {
-    modbus_message_t mode_cmd = {
+    modbus_message_t rpm_cmd = {
         .context = (void *)VFD_GetRPM,
         .crc_check = false,
         .adu[0] = vfd_config.modbus_address,
@@ -162,7 +183,19 @@ static spindle_state_t v1_spindleGetState (void)
         .rx_length = 8
     };
 
-    modbus_send(&mode_cmd, &v1_callbacks, false); // TODO: add flag for not raising alarm?
+    modbus_send(&rpm_cmd, &v1_callbacks, false); // TODO: add flag for not raising alarm?
+
+    modbus_message_t amps_cmd = {
+        .context = (void *)VFD_GetAmps,
+        .crc_check = false,
+        .adu[0] = VFD_ADDRESS,
+        .adu[1] = ModBus_ReadInputRegisters,
+        .adu[2] = 0x03,
+        .adu[3] = 0x02,     // Output amps * 10
+        .tx_length = 8,
+        .rx_length = 8
+    };
+    modbus_send(&amps_cmd, &v1_callbacks, false); // TODO: add flag for not raising alarm?
 
     // Get the actual RPM from spindle encoder input when available.
     if(hal.spindle.get_data && hal.spindle.get_data != v1_spindleGetData) {
@@ -192,6 +225,19 @@ static void v1_rx_packet (modbus_message_t *msg)
                 rpm_max50 = (msg->adu[4] << 8) | msg->adu[5];
                 break;
 
+            case VFD_GetMaxAmps:
+                amps_max = (float)((msg->adu[4] << 8) | msg->adu[5]) / 10.0f;
+                //printf("Max Amps: %.2f\n", amps_max);
+                break;
+
+            case VFD_GetAmps:
+                amps = (float)((msg->adu[4] << 8) | msg->adu[5]) / 10.0f;
+                //printf("Amps: %.2f\n", amps);
+                //printf("Load: %lu%%\n", (uint32_t)((amps/amps_max) * 100.0f));
+                if (amps_max)
+                    spindle_load = (uint8_t)((amps/amps_max) * 100.0f);
+                break;
+
             default:
                 break;
         }
@@ -208,6 +254,7 @@ bool v1_spindle_config (void)
     if(!init_ok) {
         init_ok = true;
         v1_spindleGetMaxRPM();
+        v1_spindleGetMaxAmps();
     }
 
     return true;
@@ -411,6 +458,8 @@ static void huanyang_reset (void)
 #if VFD_ENABLE == SPINDLE_HUANYANG1 || VFD_ENABLE == SPINDLE_ALL
     if(v1_active)
         v1_spindleGetMaxRPM();
+        v1_spindleGetMaxAmps();
+    }
 #endif
 #if VFD_ENABLE == SPINDLE_HUANYANG2 || VFD_ENABLE == SPINDLE_ALL
     if(v2_active)
@@ -422,6 +471,12 @@ static bool huanyang_spindle_select (spindle_id_t spindle_id)
 {
     if((v1_active = spindle_id == v1_spindle_id) || (v2_active = spindle_id == v2_spindle_id)) {
 
+        // spindle load reporting capable
+        if (v1_active)
+            has_spindle_load = true;
+        else
+            has_spindle_load = false;
+
         if(settings.spindle.ppr == 0)
             hal.spindle.get_data = spindleGetData;
 
diff --git a/vfd/spindle.c b/vfd/spindle.c
index cfd6089..c47e273 100644
--- a/vfd/spindle.c
+++ b/vfd/spindle.c
@@ -52,9 +52,13 @@ static nvs_address_t nvs_address = 0;
 
 static on_spindle_select_ptr on_spindle_select;
 static on_report_options_ptr on_report_options;
+static on_realtime_report_ptr on_realtime_report;
 
 vfd_settings_t vfd_config;
 
+bool has_spindle_load = false;
+uint8_t spindle_load = 0;
+
 spindle_id_t vfd_register (const spindle_ptrs_t *spindle, const char *name)
 {
     spindle_id_t spindle_id = -1;
@@ -161,6 +165,18 @@ static setting_details_t vfd_setting_details = {
     .save = vfd_settings_save
 };
 
+static void spindle_realtime_report (stream_write_ptr stream_write, report_tracking_flags_t report)
+{
+    if(has_spindle_load) {
+        char *load = uitoa(spindle_load);
+        stream_write("|Sl:");
+        stream_write(load);
+    }
+
+    if(on_realtime_report)
+        on_realtime_report(stream_write, report);
+}
+
 /*
 static void vfd_onReportOptions (bool newopt)
 {
@@ -193,6 +209,9 @@ void vfd_init (void)
         on_report_options = grbl.on_report_options;
 //        grbl.on_report_options = vfd_onReportOptions;
 
+        on_realtime_report = grbl.on_realtime_report;
+        grbl.on_realtime_report = spindle_realtime_report;
+
         settings_register(&vfd_setting_details);
 
 #if VFD_ENABLE == SPINDLE_ALL || VFD_ENABLE == SPINDLE_HUANYANG1 || VFD_ENABLE == SPINDLE_HUANYANG2
diff --git a/vfd/spindle.h b/vfd/spindle.h
index c4be6a1..13f6a89 100644
--- a/vfd/spindle.h
+++ b/vfd/spindle.h
@@ -55,7 +55,9 @@ typedef enum {
     VFD_GetMaxRPM,
     VFD_GetMaxRPM50,
     VFD_GetStatus,
-    VFD_SetStatus
+    VFD_SetStatus,
+    VFD_GetMaxAmps,
+    VFD_GetAmps
 } vfd_response_t;
 
 typedef struct {
@@ -75,6 +77,9 @@ typedef struct {
 
 extern vfd_settings_t vfd_config;
 
+extern bool has_spindle_load;
+extern uint8_t spindle_load;
+
 spindle_id_t vfd_register (const spindle_ptrs_t *spindle, const char *name);
 
 #endif

[terjeio]

Here is how I am thinking:

vfd.zip

This way it will be easy to add new (and optional) functionality by letting the VFD drivers telling vfd/spindle.c what they are capable of. As a bonus adding functions to vfd_ptrs_t will not break any existing drivers .

[dresco]

Here is how I am thinking:

Cool thanks, will have a look...

[dresco]

This is working for me, just a couple of small updates;

Added the power related modbus bits back for the Huanyang v1 VFD
Additional check that vfd_spindle is not null (else got load report when PWM spindle active)
Add new function to retrieve load value (e.g. from my control panel plugin)

vfd.zip

[terjeio]

I have commited my changes, can you add yours in a PR?

Additional check that vfd_spindle is not null (else got load report when PWM spindle active)

I have changed the final code a bit, should be ok now.

Add new function to retrieve load value (e.g. from my control panel plugin)

Perhaps a function to retrieve the active VFD function pointers (as a pointer to vfd_spindle) is more flexible? Then the plugin code can determine what is available and any new functions will be instantly available. A bit like how it is done in the core but with a single call?