• Tags
  • Documentation
  • Order
  • Register
  • Login
Duet3D Logo Duet3D
  • Tags
  • Documentation
  • Order
  • Register
  • Login

M308 and Current Loop Temperature Sensors

Scheduled Pinned Locked Moved
Duet Hardware and wiring
4
32
2.0k
Loading More Posts
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • undefined
    AJ Quick
    last edited by AJ Quick 4 Dec 2020, 04:14 12 Apr 2020, 03:18

    Just some oddities. I can't tell if it is working or not. Here is what the dashboard is showing me:

    heaters sensors.png

    And here are my settings:

    ; Sensors
    M308 S0 P"spi.cs5" Y"current-loop" A"Chamber" C2 L100 H500
    M308 S1 P"spi.cs5" Y"current-loop" A"Model" C0 L100 H500
    M308 S2 P"spi.cs5" Y"current-loop" A"Support" C1 L100 H500
    ; Heaters
    M140 H-1 ; Disable Heated Bed
    M950 H0 C"0x71.4" T0 ; Chamber Q what frequency?
    M950 H1 C"0x71.10" T1 ; Model Q what frequency?
    M950 H2 C"0x71.11" T2 ; Support Q what frequency?
    M143 H1 S280 P1 A2 ; Limit model to 280 degrees C maximum
    M143 H2 S280 P2 A2 ; Limit support to 280 degrees C maximum
    M141 H0 ; Set Chamber Heater
    ;M307 H0 B1 ; Enable Bitbang for Chamber
    M143 H0 S80 P0 A2 ; Limit chamber to 80 degrees C maximum
    ; Tools
    M563 P0 S"Model" D0 H1 ; Define tool 0
    G10 P0 X0 Y0 Z0 ; Set tool 0 axis offsets
    G10 P0 R0 S0 ; Set initial tool 0 active and standby temperatures to 0C
    M563 P1 S"Support" D1 H2 ; Define tool 1
    G10 P1 X0 Y0 Z0 ; Set tool 1 axis offsets
    G10 P1 R0 S0 ; Set initial tool 1 active and standby temperatures to 0C

    It appears I'm reading 0 volts, so 100 degrees is showing up for chamber? Nothing for the others.

    The actual voltages are:

    Chamber: 0.22V
    Model: 0.22V
    Support: 0.24V

    Which should go to the ADC on the various channels. (I know I've got my scales incorrect, this should be around 22-24 degrees C.)

    1 Reply Last reply Reply Quote 0
    • undefined
      AJ Quick
      last edited by 12 Apr 2020, 20:44

      Whoops. Just realized the Model and Support only report data once the temperature is over ~100 degrees. I guess I can't have those temperatures reported until the heaters are actually on.

      The Chamber should be around 600/4096 = 14.6 degrees Celsius. I believe I need to go back and calculate the range as it is now reporting a temperature of -26 degrees. It's cold in here, but it isn't THAT cold.

      1 Reply Last reply Reply Quote 0
      • undefined
        AJ Quick
        last edited by AJ Quick 14 Apr 2020, 01:40

        Now that I have everything dialed in, I did in fact start getting errors on my console. I don't know if they were always there, or if I changed something and they just started showing up.

        The problem is when assigning M308 P"spi.cs5" for each of the temperature sensors. I now get the error: "Pin 'spi.cs5' is not free".

        Presumably because you can't assign sensors to use the same logical pin as others? I'll definitely be tracking this down and addressing it in the firmware if possible.

        ? 1 Reply Last reply 14 Apr 2020, 02:58 Reply Quote 0
        • ?
          A Former User @AJ Quick
          last edited by A Former User 14 Apr 2020, 02:58

          @AJ-Quick said in M308 and Current Loop Temperature Sensors:

          I now get the error: "spi.cs5" is not free.

          not having read the whole thing, i think you need a chip select pin for each max31586 thermocouple chip?

          1 Reply Last reply Reply Quote 0
          • undefined
            AJ Quick
            last edited by 14 Apr 2020, 04:17

            This post is deleted!
            1 Reply Last reply Reply Quote 0
            • undefined
              AJ Quick
              last edited by 14 Apr 2020, 05:07

              I'm trying to work my way through this.

              If I understand correctly:

              CurrentLoopTemperatureSensor impliments SpiTemperatureSensor and that impliments SensorWithPort, which then assigns the pin inside of IoPorts.

              IoPorts.cpp is preventing the port from being re-used. However if the pin type is set to "temporaryInput", we can assign it to more than one sensor. I think it would then be possible to add a special case where CurrentLoopTemperatureSensor can be classified as a temporaryInput (though all SPI temperature sensors should probably have the ability to re-use the chipselect pins).

              1 Reply Last reply Reply Quote 0
              • undefined
                AJ Quick
                last edited by AJ Quick 14 Apr 2020, 05:26

                This may work.

                Update SensorWithPort.cpp:

                #include "CurrentLoopTemperatureSensor.h"
                // Try to configure the port. Return true if the port is valid at the end, else return false and set the error message in 'reply'. Set 'seen' if we saw the P parameter.
                bool SensorWithPort::ConfigurePort(GCodeBuffer& gb, const StringRef& reply, PinAccess access, bool& seen)
                {
                if (gb.Seen('P'))
                {
                seen = true;
                if (ReducedStringEquals(GetSensorType(),CurrentLoopTemperatureSensor::TypeName))
                {
                return port.AssignPort(gb, reply, PinUsedBy::temporaryInput, access);
                }
                else
                {
                return port.AssignPort(gb, reply, PinUsedBy::sensor, access);
                }
                }
                if (port.IsValid())
                {
                return true;
                }
                reply.copy("Missing port name parameter");
                return false;
                }

                It compiled correctly, and I'll test it tomorrow.

                1 Reply Last reply Reply Quote 0
                • undefined
                  dc42 administrators
                  last edited by 14 Apr 2020, 10:17

                  The MCP3204 support was written to support just one channel. Therefore the firmware would have to be modified to support more than one concurrently. There is a mechanism in RRF 3.01 to allow a sensor to piggyback off another one, which is how the DHT humidity sensors are set up. We wrote it to be more general than was needed for DHT sensors, so it should be possible to adapt the MCP3204 code to support it.

                  Duet WiFi hardware designer and firmware engineer
                  Please do not ask me for Duet support via PM or email, use the forum
                  http://www.escher3d.com, https://miscsolutions.wordpress.com

                  1 Reply Last reply Reply Quote 0
                  • undefined
                    AJ Quick
                    last edited by 14 Apr 2020, 16:33

                    Back in October 2018 I wrote and expanded the CurrentLoopSensor code to accept a C (channel) parameter. The MCP3204 can accept 4 signals (or 2 differential) and the MCP3208 can accept 8 signals or 4 differential. It's in the code already.

                    I've done a little digging and did see the code for the parent and child sensors, but have not yet figured out how they would be implemented. Potentially that would be the best if they could be assigned that way instead of writing an exception to the SensorsWithPorts code. I'll look into it later, but I'll be testing my most recent change to see how if it works.

                    1 Reply Last reply Reply Quote 0
                    • undefined
                      AJ Quick
                      last edited by AJ Quick 15 Apr 2020, 02:53

                      I don't think the parent / child would be a bad idea. However, I am not sure exactly how it would be best to implement.

                      Perhaps if we had "current-loop" as one option and then "current-loop-child" as another?

                      Would it make sense to assign the channel through the "P" command?

                      My understanding is it could be assigned as follows:

                      M308 S0 P"spi.cs5" Y"current-loop" A"Channel 0" ; Current Loop Sensor by default on Channel 0.
                      M308 S1 P"S0.1" Y"current-loop-child" A"Channel 1" ; Current Loop Child on Channel 1
                      M308 S2 P"S0.7" Y"current-loop-child" A"Channel 7" ; Current Loop Child on Channel 7

                      OR

                      M308 S0 P"spi.cs5" Y"current-loop" A"Channel 7" C7 ; Current Loop Sensor on Channel 7.
                      M308 S1 P"S0.1" Y"current-loop-child" A"Channel 5" C5 ; Current Loop Child on Channel 5
                      M308 S2 P"S0.2" Y"current-loop-child" A"Channel 6" C6 ; Current Loop Child on Channel 6

                      I think the second example is better as not everyone is going to automatically be using Channel 0. Perhaps this can be done without needing to call it 'current-loop-child' but instead just checking if the pin is already in use by a current loop sensor? If it is in use, then apply the 'AdditionalOutputSensor'?

                      undefined 1 Reply Last reply 17 Apr 2020, 06:18 Reply Quote 0
                      • undefined
                        AJ Quick
                        last edited by 17 Apr 2020, 01:53

                        Unsurprisingly, my possible fix (M308 and Current Loop Temperature Sensors) did not work.

                        I did not understand how the port was being assigned correctly.

                        I'll look more into this parent / child relationship and see what I can accomplish.

                        1 Reply Last reply Reply Quote 0
                        • undefined
                          dc42 administrators @AJ Quick
                          last edited by 17 Apr 2020, 06:18

                          @AJ-Quick said in M308 and Current Loop Temperature Sensors:

                          I don't think the parent / child would be a bad idea. However, I am not sure exactly how it would be best to implement.

                          Perhaps if we had "current-loop" as one option and then "current-loop-child" as another?

                          Would it make sense to assign the channel through the "P" command?

                          My understanding is it could be assigned as follows:

                          M308 S0 P"spi.cs5" Y"current-loop" A"Channel 0" ; Current Loop Sensor by default on Channel 0.
                          M308 S1 P"S0.1" Y"current-loop-child" A"Channel 1" ; Current Loop Child on Channel 1
                          M308 S2 P"S0.7" Y"current-loop-child" A"Channel 7" ; Current Loop Child on Channel 7

                          OR

                          M308 S0 P"spi.cs5" Y"current-loop" A"Channel 7" C7 ; Current Loop Sensor on Channel 7.
                          M308 S1 P"S0.1" Y"current-loop-child" A"Channel 5" C5 ; Current Loop Child on Channel 5
                          M308 S2 P"S0.2" Y"current-loop-child" A"Channel 6" C6 ; Current Loop Child on Channel 6

                          I think the second example is better as not everyone is going to automatically be using Channel 0. Perhaps this can be done without needing to call it 'current-loop-child' but instead just checking if the pin is already in use by a current loop sensor? If it is in use, then apply the 'AdditionalOutputSensor'?

                          The first option would fit better with the architecture and I think it would be easier to implement.

                          Duet WiFi hardware designer and firmware engineer
                          Please do not ask me for Duet support via PM or email, use the forum
                          http://www.escher3d.com, https://miscsolutions.wordpress.com

                          undefined 1 Reply Last reply 17 Apr 2020, 06:32 Reply Quote 0
                          • undefined
                            AJ Quick @dc42
                            last edited by 17 Apr 2020, 06:32

                            @dc42 said in M308 and Current Loop Temperature Sensors:

                            The first option would fit better with the architecture and I think it would be easier to implement.

                            Do you believe it could be done without calling it 'current-loop-child' (or a variant)? Or would something need to be made to differentiate it from a regular current-loop sensor?

                            undefined 1 Reply Last reply 17 Apr 2020, 16:22 Reply Quote 0
                            • undefined
                              dc42 administrators @AJ Quick
                              last edited by dc42 17 Apr 2020, 16:22

                              @AJ-Quick said in M308 and Current Loop Temperature Sensors:

                              Do you believe it could be done without calling it 'current-loop-child' (or a variant)? Or would something need to be made to differentiate it from a regular current-loop sensor?

                              Yes, it needs a different sensor type name from the sensor that it piggybacks on to, because the name identifies which driver class it uses. Could be current-loop-additional or current-loop-extra or something like that. I suggest you keep "current-loop" in the name to make it clear which master sensor type it needs.

                              Duet WiFi hardware designer and firmware engineer
                              Please do not ask me for Duet support via PM or email, use the forum
                              http://www.escher3d.com, https://miscsolutions.wordpress.com

                              1 Reply Last reply Reply Quote 0
                              • undefined
                                AJ Quick
                                last edited by AJ Quick 18 Apr 2020, 03:10

                                I made the following changes:

                                https://github.com/dc42/RepRapFirmware/pull/380

                                I can't imagine it is that easy though. This does add 'current-loop-extra' which then (hopefully) directs to AdditionalOutputSensor. I am assuming all parameters that are passed, won't be passed to the parent though?

                                So if the "C" parameter is working now, it should work to set the different "C" parameter on the child? It won't force the channel to be adopted by the parent?

                                This possibly works like this now:

                                M308 S0 P"spi.cs5" Y"current-loop" A"Channel 7" C7 ; Current Loop Sensor on Channel 7.
                                M308 S1 P"S0.1" Y"current-loop-extra" A"Channel 5" C5 ; Current Loop Child on Channel 5
                                M308 S2 P"S0.2" Y"current-loop-extra" A"Channel 6" C6 ; Current Loop Child on Channel 6

                                Up to 7 children can be defined.

                                In order for the P parameter to carry the channel data, the existing code would need to be changed.

                                ajquick opened this pull request 18 Apr 2020, 04:45 in dc42/RepRapFirmware

                                draft CurrentLoopSensors can now have children sensors. #380

                                1 Reply Last reply Reply Quote 0
                                • undefined
                                  dc42 administrators
                                  last edited by 18 Apr 2020, 07:26

                                  Do you need the C parameter at all? I would have thought that the master current loop sensor would use channel 0, then child 1 (M308 S2 P"S0.1") would use channel 1, and so on.

                                  Duet WiFi hardware designer and firmware engineer
                                  Please do not ask me for Duet support via PM or email, use the forum
                                  http://www.escher3d.com, https://miscsolutions.wordpress.com

                                  undefined 1 Reply Last reply 19 Apr 2020, 17:44 Reply Quote 0
                                  • undefined
                                    AJ Quick @dc42
                                    last edited by 19 Apr 2020, 17:44

                                    @dc42 Yes.

                                    While channel 0 is the default, you can't assume all end users will have their main temperature probe set on channel 0. My main temperature reading is on channel 2 for example.

                                    To force the channel to be designated by the P parameter, you lose backwards compatibility and need to change more of the default CurrentLoop code. Since you need to specify L and H settings anyways, it makes sense to just continue to use the C (and D) parameters for the child as well.

                                    1 Reply Last reply Reply Quote 0
                                    • undefined
                                      AJ Quick
                                      last edited by 19 Apr 2020, 19:01

                                      Current status:

                                      Current-Loop-Extra currently works by assigning a child of the parent sensor. Unfortunately the child is an exact clone of the parent. No channel, differential or low and high parameters are passed.

                                      This might just be a problem with "AdditionalOutputSensor.cpp" though. It probably only passes the "Y" parameter currently.

                                      C/D/L/H cannot be assumed to be the same for the parent and the child. For example my thermocouples have different L/H ranges.

                                      1 Reply Last reply Reply Quote 0
                                      • undefined
                                        AJ Quick
                                        last edited by AJ Quick 20 Apr 2020, 03:21

                                        At a total loss here.

                                        I am now understanding how the DHT sensor works, and it is not the same as how the MCP3204/8 works. DHT seems to always have the humidity value present, so it is a simple matter to just ask what that value is.

                                        MCP3204/8 needs to treat each sensor as their own and communicate with the chip for each and every one. It also isn't something you can just change the channel and grab, since each sensor has its settings. I tried to allow AdditionalOutputSensor to configure the child sensor, but it just overrides the parent.

                                        I suppose it would be possible to fully expand the CurrentLoopExtraSensor class to basically be a copy of CurrentLoopSensor, but that's not clean. There would be lots of code re-use.

                                        The best thing I think that can be done is just to allow the SPI chipselect to be used by multiple sensors as long as the sensor is of the same type. At this time, I do not know how to accomplish that. I believe it should be safe to re-use chipselect pins, because they will automatically be queued and wouldn't be talking over each other. The worst case scenario is someone puts in the wrong chipselect and it cannot connect.

                                        1 Reply Last reply Reply Quote 0
                                        • undefined
                                          AJ Quick
                                          last edited by 20 Apr 2020, 04:42

                                          This is what I've come up with:

                                          https://github.com/dc42/RepRapFirmware/pull/380/commits/2ca324d3fd53f2988b4e2d95e878a2347b4065ee

                                          Basically, add a new pin type of "chipSelect".

                                          If it is of type chipSelect, it can be shared. When an SPI sensor makes a port, it assigns as type chipSelect. Now only other sensors can use that port for chip-select if it was previously used for chip-select. It should throw an error if any other code tries to use it for something other than chip-select.

                                          This should allow CurrentLoopSensor to work while sharing the same chip-select and potentially can be used for other chips down the line.

                                          ajquick opened this pull request 18 Apr 2020, 04:45 in dc42/RepRapFirmware

                                          draft CurrentLoopSensors can now have children sensors. #380

                                          1 Reply Last reply Reply Quote 0
                                          11 out of 32
                                          • First post
                                            11/32
                                            Last post
                                          Unless otherwise noted, all forum content is licensed under CC-BY-SA