Merge pull request #732 from wolfmanjm/refactor/get-public-data-more

refactor get public data, to be slightly easier to use on callee side

diff --git a/src/libs/PublicData.cpp b/src/libs/PublicData.cpp
index 3307d00..28401dc 100644 (file)
--- a/src/libs/PublicData.cpp
+++ b/src/libs/PublicData.cpp
@@ -4,7 +4,9 @@
 
 bool PublicData::get_value(uint16_t csa, uint16_t csb, uint16_t csc, void *data) {
     PublicDataRequest pdr(csa, csb, csc);
-    pdr.set_data_ptr(data); // the caller may have put a placeholder for the returned data here
+    // the caller may have created the storage for the returned data so we clear the flag,
+    // if it gets set by the callee setting the data ptr that means the data is a pointer to a pointer and is set to a pointer to the returned data
+    pdr.set_data_ptr(data, false);
     THEKERNEL->call_event(ON_GET_PUBLIC_DATA, &pdr );
     if(pdr.is_taken() && pdr.has_returned_data()) {
         // the callee set the returned data pointer

diff --git a/src/libs/PublicData.h b/src/libs/PublicData.h
index 911b7a3..a888e9e 100644 (file)
--- a/src/libs/PublicData.h
+++ b/src/libs/PublicData.h
@@ -10,6 +10,10 @@
 
 class PublicData {
     public:
+        // there are two ways to get data from a module
+        // 1. pass in a pointer to a data storage area that the caller creates, the callee module will put the returned data in that pointer
+        // 2. pass in a pointer to a pointer, the callee will set that pointer to some storage the callee has control over, with the requested data
+        // the version used is dependent on the target (callee) module
         static bool get_value(uint16_t csa, void *data) { return get_value(csa, 0, 0, data); }
         static bool get_value(uint16_t csa, uint16_t csb, void *data) { return get_value(csa, csb, 0, data); }
         static bool get_value(uint16_t cs[3], void *data) { return get_value(cs[0], cs[1], cs[2], data); };

diff --git a/src/libs/PublicDataRequest.h b/src/libs/PublicDataRequest.h
index 5ff7efd..aabb449 100644 (file)
--- a/src/libs/PublicDataRequest.h
+++ b/src/libs/PublicDataRequest.h
@@ -14,7 +14,7 @@ class PublicDataRequest {
         PublicDataRequest(uint16_t addrcs1, uint16_t addrcs2){ target[0]= addrcs1; target[1]= addrcs2; target[2]= 0; data_taken= false; data= NULL; returned_data= true; }
         PublicDataRequest(uint16_t addrcs1, uint16_t addrcs2, uint16_t addrcs3){ target[0]= addrcs1; target[1]= addrcs2; target[2]= addrcs3; data_taken= false; data= NULL; returned_data= true; }
 
-        virtual ~PublicDataRequest() { data= NULL; }
+        virtual ~PublicDataRequest() { data= nullptr; }
 
         bool starts_with(uint16_t addr) const { return addr == this->target[0]; }
         bool second_element_is(uint16_t addr) const { return addr == this->target[1]; }
@@ -22,9 +22,8 @@ class PublicDataRequest {
 
         bool is_taken() const { return this->data_taken; }
         void set_taken() { this->data_taken= true; }
-        void clear_returned_data() {this->returned_data= false; }
         bool has_returned_data() const { return this->returned_data; }
-        void set_data_ptr(void *d) { this->data= d; }
+        void set_data_ptr(void *d, bool flag= true) { this->data= d; returned_data= flag; }
         void* get_data_ptr(void) const { return this->data; }
 
     private:

diff --git a/src/modules/robot/Robot.cpp b/src/modules/robot/Robot.cpp
index 62b8c4a..b1f1643 100644 (file)
--- a/src/modules/robot/Robot.cpp
+++ b/src/modules/robot/Robot.cpp
@@ -23,7 +23,6 @@ using std::string;
 #include "Gcode.h"
 #include "PublicDataRequest.h"
 #include "PublicData.h"
-#include "RobotPublicAccess.h"
 #include "arm_solutions/BaseSolution.h"
 #include "arm_solutions/CartesianSolution.h"
 #include "arm_solutions/RotatableCartesianSolution.h"
@@ -279,58 +278,6 @@ void Robot::on_halt(void *arg)
     halted= (arg == nullptr);
 }
 
-void Robot::on_get_public_data(void *argument)
-{
-    PublicDataRequest *pdr = static_cast<PublicDataRequest *>(argument);
-
-    if(!pdr->starts_with(robot_checksum)) return;
-
-    if(pdr->second_element_is(speed_override_percent_checksum)) {
-        static float return_data;
-        return_data = 100.0F * 60.0F / seconds_per_minute;
-        pdr->set_data_ptr(&return_data);
-        pdr->set_taken();
-
-    } else if(pdr->second_element_is(current_position_checksum)) {
-        static float return_data[3];
-        return_data[0] = from_millimeters(this->last_milestone[0]);
-        return_data[1] = from_millimeters(this->last_milestone[1]);
-        return_data[2] = from_millimeters(this->last_milestone[2]);
-
-        pdr->set_data_ptr(&return_data);
-        pdr->set_taken();
-    }
-}
-
-void Robot::on_set_public_data(void *argument)
-{
-    PublicDataRequest *pdr = static_cast<PublicDataRequest *>(argument);
-
-    if(!pdr->starts_with(robot_checksum)) return;
-
-    if(pdr->second_element_is(speed_override_percent_checksum)) {
-        // NOTE do not use this while printing!
-        float t = *static_cast<float *>(pdr->get_data_ptr());
-        // enforce minimum 10% speed
-        if (t < 10.0F) t = 10.0F;
-
-        this->seconds_per_minute = t / 0.6F; // t * 60 / 100
-        pdr->set_taken();
-    } else if(pdr->second_element_is(current_position_checksum)) {
-        float *t = static_cast<float *>(pdr->get_data_ptr());
-        for (int i = 0; i < 3; i++) {
-            this->last_milestone[i] = this->to_millimeters(t[i]);
-        }
-
-        float actuator_pos[3];
-        arm_solution->cartesian_to_actuator(last_milestone, actuator_pos);
-        for (int i = 0; i < 3; i++)
-            actuators[i]->change_last_milestone(actuator_pos[i]);
-
-        pdr->set_taken();
-    }
-}
-
 //A GCode has been received
 //See if the current Gcode line has some orders for us
 void Robot::on_gcode_received(void *argument)

diff --git a/src/modules/robot/Robot.h b/src/modules/robot/Robot.h
index 1641912..0063ef7 100644 (file)
--- a/src/modules/robot/Robot.h
+++ b/src/modules/robot/Robot.h
@@ -26,8 +26,6 @@ class Robot : public Module {
         void on_module_loaded();
         void on_config_reload(void* argument);
         void on_gcode_received(void* argument);
-        void on_get_public_data(void* argument);
-        void on_set_public_data(void* argument);
         void on_halt(void *arg);
 
         void reset_axis_position(float position, int axis);

diff --git a/src/modules/robot/RobotPublicAccess.h b/src/modules/robot/RobotPublicAccess.h
deleted file mode 100644 (file)
index 0255f5c..0000000
--- a/src/modules/robot/RobotPublicAccess.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef __ROBOTPUBLICACCESS_H_
-#define __ROBOTPUBLICACCESS_H_
-
-// addresses used for public data access
-#define robot_checksum                         CHECKSUM("robot")
-#define speed_override_percent_checksum        CHECKSUM("speed_override_percent")
-#define current_position_checksum              CHECKSUM("current_position")
-
-#endif
\ No newline at end of file

diff --git a/src/modules/tools/switch/Switch.cpp b/src/modules/tools/switch/Switch.cpp
index eeafb14..8b7c3aa 100644 (file)
--- a/src/modules/tools/switch/Switch.cpp
+++ b/src/modules/tools/switch/Switch.cpp
@@ -302,14 +302,12 @@ void Switch::on_get_public_data(void *argument)
     if(!pdr->second_element_is(this->name_checksum)) return; // likely fan, but could be anything
 
     // ok this is targeted at us, so send back the requested data
-    // caller has provided the location to write the state
+    // caller has provided the location to write the state to
     struct pad_switch *pad= static_cast<struct pad_switch *>(pdr->get_data_ptr());
     pad->name = this->name_checksum;
     pad->state = this->switch_state;
     pad->value = this->switch_value;
-
     pdr->set_taken();
-    pdr->clear_returned_data();
 }
 
 void Switch::on_set_public_data(void *argument)

diff --git a/src/modules/tools/temperaturecontrol/TemperatureControl.cpp b/src/modules/tools/temperaturecontrol/TemperatureControl.cpp
index 071d3d7..b263c75 100644 (file)
--- a/src/modules/tools/temperaturecontrol/TemperatureControl.cpp
+++ b/src/modules/tools/temperaturecontrol/TemperatureControl.cpp
@@ -368,12 +368,11 @@ void TemperatureControl::on_get_public_data(void *argument)
         t.id= this->name_checksum;
         v->push_back(t);
         pdr->set_taken();
-        pdr->clear_returned_data();
 
     }else if(pdr->second_element_is(current_temperature_checksum)) {
         // if targeted at us
         if(pdr->third_element_is(this->name_checksum)) {
-            // ok this is targeted at us, so send back the requested data
+            // ok this is targeted at us, so set the requ3sted data in the pointer passed into us
             struct pad_temperature *t= static_cast<pad_temperature*>(pdr->get_data_ptr());
             t->current_temperature = this->get_temperature();
             t->target_temperature = (target_temperature <= 0) ? 0 : this->target_temperature;
@@ -381,7 +380,6 @@ void TemperatureControl::on_get_public_data(void *argument)
             t->designator= this->designator;
             t->id= this->name_checksum;
             pdr->set_taken();
-            pdr->clear_returned_data();
         }
     }
 

diff --git a/src/modules/tools/toolmanager/ToolManager.cpp b/src/modules/tools/toolmanager/ToolManager.cpp
index 2dd5d65..7aea92c 100644 (file)
--- a/src/modules/tools/toolmanager/ToolManager.cpp
+++ b/src/modules/tools/toolmanager/ToolManager.cpp
@@ -26,8 +26,6 @@ using namespace std;
 #include "libs/StreamOutput.h"
 #include "FileStream.h"
 
-#include "modules/robot/RobotPublicAccess.h"
-
 #define return_error_on_unhandled_gcode_checksum    CHECKSUM("return_error_on_unhandled_gcode")
 
 ToolManager::ToolManager(){

diff --git a/src/modules/utils/panel/screens/ControlScreen.cpp b/src/modules/utils/panel/screens/ControlScreen.cpp
index 9a870d2..53ab8ad 100644 (file)
--- a/src/modules/utils/panel/screens/ControlScreen.cpp
+++ b/src/modules/utils/panel/screens/ControlScreen.cpp
@@ -14,7 +14,7 @@
 #include "libs/nuts_bolts.h"
 #include "libs/utils.h"
 #include <string>
-#include "modules/robot/RobotPublicAccess.h"
+#include "Robot.h"
 #include "PublicData.h"
 #include "checksumm.h"
 #include "LcdBase.h"
@@ -125,15 +125,7 @@ void ControlScreen::enter_menu_control()
 
 void ControlScreen::get_current_pos(float *cp)
 {
-    void *returned_data;
-
-    bool ok = PublicData::get_value( robot_checksum, current_position_checksum, &returned_data );
-    if (ok) {
-        float *p = static_cast<float *>(returned_data);
-        cp[0] = p[0];
-        cp[1] = p[1];
-        cp[2] = p[2];
-    }
+    THEKERNEL->robot->get_axis_position(cp);
 }
 
 void ControlScreen::set_current_pos(char axis, float p)

diff --git a/src/modules/utils/panel/screens/WatchScreen.cpp b/src/modules/utils/panel/screens/WatchScreen.cpp
index 09e077c..b9ec420 100644 (file)
--- a/src/modules/utils/panel/screens/WatchScreen.cpp
+++ b/src/modules/utils/panel/screens/WatchScreen.cpp
@@ -14,7 +14,7 @@
 #include "libs/nuts_bolts.h"
 #include "libs/utils.h"
 #include "modules/tools/temperaturecontrol/TemperatureControlPublicAccess.h"
-#include "modules/robot/RobotPublicAccess.h"
+#include "Robot.h"
 #include "modules/robot/Conveyor.h"
 #include "modules/utils/player/PlayerPublicAccess.h"
 #include "NetworkPublicAccess.h"
@@ -206,27 +206,13 @@ void WatchScreen::get_current_status()
 // fetch the data we are displaying
 float WatchScreen::get_current_speed()
 {
-    void *returned_data;
-
-    bool ok = PublicData::get_value( robot_checksum, speed_override_percent_checksum, &returned_data );
-    if (ok) {
-        float cs = *static_cast<float *>(returned_data);
-        return cs;
-    }
-    return 0.0;
+    // in percent
+    return 6000.0F / THEKERNEL->robot->get_seconds_per_minute();
 }
 
 void WatchScreen::get_current_pos(float *cp)
 {
-    void *returned_data;
-
-    bool ok = PublicData::get_value( robot_checksum, current_position_checksum, &returned_data );
-    if (ok) {
-        float *p = static_cast<float *>(returned_data);
-        cp[0] = p[0];
-        cp[1] = p[1];
-        cp[2] = p[2];
-    }
+    THEKERNEL->robot->get_axis_position(cp);
 }
 
 void WatchScreen::get_sd_play_info()

diff --git a/src/modules/utils/simpleshell/SimpleShell.cpp b/src/modules/utils/simpleshell/SimpleShell.cpp
index 12e948d..d904170 100644 (file)
--- a/src/modules/utils/simpleshell/SimpleShell.cpp
+++ b/src/modules/utils/simpleshell/SimpleShell.cpp
@@ -22,10 +22,9 @@
 #include "checksumm.h"
 #include "PublicData.h"
 #include "Gcode.h"
-//#include "StepTicker.h"
+#include "Robot.h"
 
 #include "modules/tools/temperaturecontrol/TemperatureControlPublicAccess.h"
-#include "modules/robot/RobotPublicAccess.h"
 #include "NetworkPublicAccess.h"
 #include "platform_memory.h"
 #include "SwitchPublicAccess.h"
@@ -588,15 +587,9 @@ void SimpleShell::get_command( string parameters, StreamOutput *stream)
         }
 
     } else if (what == "pos") {
-        float *pos;
-        bool ok = PublicData::get_value( robot_checksum, current_position_checksum, &pos );
-
-        if (ok) {
-             stream->printf("Position X: %f, Y: %f, Z: %f\r\n", pos[0], pos[1], pos[2]);
-
-        } else {
-            stream->printf("get pos command failed\r\n");
-        }
+        float pos[3];
+        THEKERNEL->robot->get_axis_position(pos);
+        stream->printf("Position X: %f, Y: %f, Z: %f\r\n", pos[0], pos[1], pos[2]);
     }
 }